fix(core): enhance validation for continuous task dependencies #31786
Conversation
Improve assertTaskGraphDoesNotContainInvalidTargets to validate two scenarios: - Non-parallel tasks that depend on continuous tasks - Non-parallel continuous tasks that are depended on by other tasks Signed-off-by: AgentEnder <craigorycoppola@gmail.com>
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
View your CI Pipeline Execution ↗ for commit 4c8e4e8.
Nx Cloud AI FixNx Cloud AI analyzes your failing CI tasks and automatically generates fixes whenever possible.
☁️ Nx Cloud last updated this comment at |
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| for (const dependency of taskGraph.continuousDependencies[task.id]) { | ||
| if (taskGraph.tasks[dependency].parallelism === false) { | ||
| nonParallelContinousTasksThatAreDependedOn.push( | ||
| taskGraph.tasks[dependency] | ||
| ); | ||
| } |
There was a problem hiding this comment.
The loop checking for non-parallel continuous tasks is nested inside the previous conditional block, which means it only executes when the current task has parallelism === false and has continuous dependencies. This will miss scenarios where a task with parallelism === true depends on a continuous task with parallelism === false.
To catch all cases where a continuous task with parallelism === false is depended on, this loop should be moved outside the previous if statement but kept within the main for loop that iterates through all tasks.
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
Co-authored-by: graphite-app[bot] <96075541+graphite-app[bot]@users.noreply.github.com>
Co-authored-by: graphite-app[bot] <96075541+graphite-app[bot]@users.noreply.github.com>
| @@ -159,19 +159,33 @@ export function validateNoAtomizedTasks( | |||
| export function assertTaskGraphDoesNotContainInvalidTargets( | |||
| taskGraph: TaskGraph | |||
| ) { | |||
| const invalidTasks = []; | |||
| const nonParallelTasksThatDependOnContinuousTasks = []; | |||
| const nonParallelContinuousTasksThatAreDependedOn = []; | |||
There was a problem hiding this comment.
There's a typo in the variable name nonParallelContinousTasksThatAreDependedOn (missing the letter 'u' in "Continuous"). This misspelling appears in multiple places in the file. The correct spelling should be nonParallelContinuousTasksThatAreDependedOn.
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
Current Behavior
We error if a non-parallel task has a continuous dependency, but when running a command that contains a task that depends on a continuous task which itself is marked as non-parallel we do not error. This results in the command hanging on the continuous non-parallel task, instead of running the parent task as well.
Expected Behavior
We error in both situations.
Related Issue(s)
Fixes #