-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Start adding a new rerun order #2067
base: main
Are you sure you want to change the base?
Conversation
@@ -34,13 +36,15 @@ interface IParseGherkinMessageStreamRequest { | |||
* @param eventDataCollector | |||
* @param gherkinMessageStream | |||
* @param order | |||
* @param unexpandedFeaturePaths | |||
* @param pickleFilter | |||
*/ | |||
export async function parseGherkinMessageStream({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function is deprecated and will be removed in a future major release, however in the meantime it'd be better to avoid making a breaking change to it. Could we make the new field optional?
(It's fine to change orderPickles
, that's an internal one.)
I'm confused why this is needed. I would think if order is "defined", the order of the tests as defined in the rerun file should be respected. Can you better outline what the current case is? Is the current order based only at the file level and not at the pickle level? I don't think this should be a new order and just feels like a more proper implementation of the "defined" order (ie the order of file / file & line references passed) |
The original purpose of the rerun file is to rerun just the failing scenarios. Users might still want to run them in random order. Or perhaps they want to run them in the defined order (alphabetically ordered files, scenarios top-to-bottom). Filtering and ordering are two orthogonal concepts, so I think they should be kept separate. That said, I wouldn’t object to defaulting to |
Rerun still feels like a bad name to me for the name of this order. We have defined a rerun format as being one where each line is a number of file / line combinations that point to file / scenarios / scenario outlines. But you could also provide that in the exact same way as a number of CLI arguments. The current "defined" isn't very exact. I think its currently the following:
Examples
The second example seems wrong to me as ideally we'd run what the input was. If we fixed that, it would give you the exact order you have implemented as |
Okay, after looking at trying to implement the fix I described, believe that isn't worth it. I still don't think we should tie the name of this to "rerun". I think my ideal would be something like:
Thoughts? Note, I'm also happy to help with these changes |
`Cannot use rerun order because ${pathB} was not in the rerun order. Did you forget to specify @rerun.txt?` | ||
) | ||
} | ||
return indexA - indexB |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note I think this could be something like
const ordered = new Array(unexpandedFeaturePaths.length);
picklesWithDocument.forEach(item => {
// get index in unexpandedFeaturePaths, throw if not found
ordered[index] = item
})
// remove empty indexes (only occurs if have duplicates in list and we just use the first)
That should be O(n) instead O(nlogn)
🤔 What's changed?
This adds a new
rerun
order that can be used with the--order
option.⚡️ What's your motivation?
We are building a test case prioritization tool that can predict the tests that are most likely to fail. Our tool generates a
rerun.txt
file. Users can then use this order to run tests in the predicted order, increasing the probability of seeing test failures earlier in the test run.Currently, specifying a
rerun.txt
file will filter scenarios, but the order in which they are run is still dictated by the--order
option (defaulting todefined
).The proposed
rerun
order will use the contents of thererun.txt
file as the desired order to run tests in.🏷️ What kind of change is this?
♻️ Anything particular you want feedback on?
📋 Checklist:
This text was originally generated from a template, then edited by hand. You can modify the template here.