Skip to content

Conversation

@vkarak
Copy link
Contributor

@vkarak vkarak commented Aug 5, 2022

This PR introduces a new command line option, --exec-order, that will impose an order of execution on the independent tests. This is particularly useful when you want to randomise the order of tests that have been generated with either the --repeat or the --distribute option.

Although, currently, the order of execution is implementation defined and we could simply shuffle the tests, I decided that it's better introducing a new option for a couple of reasons:

  1. Shuffling the tests as an implementation detail could very easily change in the future and break the "expected" behaviour.
  2. As the various actions like --list and --run happen after the test processing and after the test DAG is built, shuffling the tests, implies that the listing would also be shuffled, which is not very helpful for two reasons: (a) it's not easy to find a test in a listing (b) break the current expectation that the listing order follows the (serial) execution order.
  3. Having an option to control the order makes things clearer both from the implementation's point of view but also from the user perspective.

Implementation details

Currently, all the DAG manipulation routines and the topological sorting respect the order of the test cases that are passed as input to them, which means that simply ordering or shuffling the test cases beforehand is enough. However, there is the subtle case we need to exclude some test cases due to unresolved dependencies. We were doing that by subtracting two sets, which randomised the test cases in a new way. By replacing these sets with ordered sets, the problem is solved.

Fixes #2548.

@codecov-commenter
Copy link

Codecov Report

Merging #2575 (ca5b79b) into master (7cdead0) will increase coverage by 0.01%.
The diff coverage is 94.73%.

@@            Coverage Diff             @@
##           master    #2575      +/-   ##
==========================================
+ Coverage   86.34%   86.36%   +0.01%     
==========================================
  Files          59       59              
  Lines       10892    10906      +14     
==========================================
+ Hits         9405     9419      +14     
  Misses       1487     1487              
Impacted Files Coverage Δ
reframe/frontend/cli.py 72.41% <94.73%> (+0.77%) ⬆️
reframe/frontend/executors/policies.py 92.30% <0.00%> (-0.29%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@teojgo
Copy link
Contributor

teojgo commented Aug 5, 2022

@vkarak do you think instead of the random option it would make sense to allow passing an integer value which would then be treated as the seed for the shuffling?

@vkarak
Copy link
Contributor Author

vkarak commented Aug 5, 2022

@teojgo I was thinking to keep it simple for the moment. If we see the need for a seed, then we extend the arguments that are accepted.

Copy link
Contributor

@victorusu victorusu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@vkarak vkarak merged commit 0463226 into reframe-hpc:master Aug 9, 2022
@vkarak vkarak deleted the feat/randomize-exec-order branch August 9, 2022 06:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RFE: randomize test execution order

5 participants