DEVPROD-11189 Resolve execution and display tasks correctly for API Patches #8389
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ZackarySantana
force-pushed
the
DEVPROD-11189
branch
from
12fc1aa to
6ba7303
Compare
ZackarySantana
changed the title
Kimchelly
reviewed
Dec 3, 2024
| @@ -224,3 +224,46 @@ func TestDownstreamTasks(t *testing.T) { | |||
| assert.Len(a.DownstreamTasks[0].Tasks, 2) | |||
| assert.Len(a.DownstreamTasks[0].VariantTasks, 1) | |||
| } | |||
|
|
|||
| func TestPreselectedDisplayTasks(t *testing.T) { | |||
There was a problem hiding this comment.
Would it be worth testing how it behaves where the user only selects a subset of execution tasks in the display task? I'm not sure if anyone really selects just a subset of a display task to run, but I don't think it's disallowed either, and may count as surprising/unexpected behavior.
There was a problem hiding this comment.
The execution tasks shouldn't be allowed to select on the UI (though I think the configure route could get behind that).
This test I think tests that though, it selects an execution tasks and makes sure the corresponding display task and other execution tasks are also paired (which propagates up to the variant tasks)
Kimchelly
approved these changes
Dec 3, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
DEVPROD-11189
Description
When running
evergreen patch -t all -v allyou see display tasks + their execution tasks. This is because our APIPatch's were resolved without the display tasks being linked to their execution tasks and our new UI simply displaying whatever tasks are inside the variant tasks (which would be the execution tasks as well).This PR adds the execution tasks to display tasks when we create the patch, and it also removes the execution tasks from the variant tasks in favor of just adding the display tasks.
Testing
Old patch
New patch
The "execution-task" tasks are not in the new patch. I tried scheduling both and a variety of selecting and unselecting, and the new patch appropriately schedules the execution tasks if the display task is selected.
Note that the old UI always works, because it does specific logic on its end to remove the execution tasks that are associated with the display tasks from the array before display it. This could work for the new ui, but the graphql mutations and adjustments to its data models are complicated since it reaches a lot of different places. It's simpler to do it this way because we resolve display tasks to its execution tasks anyways.