-
Notifications
You must be signed in to change notification settings - Fork 11
DAG-1992 Generate only burn_in tasks when --burn-in is passed #47
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
Conversation
a161ae1
to
9bfe93c
Compare
9bfe93c
to
0e294ee
Compare
Example of patch where burn_in and other tasks are generated separately: https://spruce.mongodb.com/version/6308dc350305b95cf9de852b Note: It is not allowed to use generate.tasks more than once in a single evergreen task. |
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.
Overall LGTM after adding tests
// handle them. | ||
if task.name == BURN_IN_TESTS { | ||
if self.gen_burn_in { | ||
if self.gen_burn_in { |
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.
Can you add some tests for this logic to make sure we're skipping the tasks when we should be?
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 particular function seems hard to test or split. I've added end-to-end test for this logic.
Also added a comment to make it more clear:
https://github.com/mongodb/mongo-task-generator/pull/47/files#diff-7ff815b68943f08bef79f083c09e55ca8c18db8e25033034b12fa31f1d43dab4R64-R65
Let me know if you have better ideas.
We should update the docs as well for this option |
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.
LGMT mod my comments
docs/generating_tasks.md
Outdated
task to the required buildvariant, that will generate another required buildvariants on the fly | ||
based on another non-required buildvariants with only necessary burn-in tasks. |
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.
[nit] This way we can burn_in on the the requested buildvariant as well other, additional variants to ensure there is no difference between variants.
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.
updated
evergreen merge |
When
--burn-in
is passed, only burn_in tasks are generated. Otherwise all the other tasks are generated.