-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Fix gradle test gap #31313
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
base: master
Are you sure you want to change the base?
Fix gradle test gap #31313
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
View your CI Pipeline Execution ↗ for commit d972df3.
☁️ Nx Cloud last updated this comment at |
🐳 We have a release for that!This PR has a release associated with it. You can try it out using this command: npx [email protected] my-workspace Or just copy this version and use it in your own command: 0.0.0-pr-31313-387cdca
To request a new release for this pull request, mention someone from the Nx team or the |
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.
Pull Request Overview
This PR refactors how Gradle tasks are processed in NX, introducing an atomized
mode for more granular CI test targets and enabling exclusion of tasks in parallel batch runs.
- Replace the
cwd
flag with a booleanatomized
parameter across project-graph utilities, reporting tasks, and plugins - Add
getExcludeTasks
and update batch-runner CLI/runner to accept--excludeTasks
and run tasks concurrently with coroutines - Enhance CI test target naming by extracting package-qualified class names and update related tests
Reviewed Changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
packages/gradle/src/executors/gradle/get-exclude-task.ts | New utilities getExcludeTasks and getAllDependsOn |
packages/gradle/project-graph/src/test/kotlin/dev/nx/gradle/utils/ProcessTaskUtilsTest.kt | Update test to expect "<project>:<task>" format |
packages/gradle/project-graph/src/test/kotlin/dev/nx/gradle/utils/CreateNodeForProjectTest.kt | Switch from cwd override to atomized flag in test |
packages/gradle/project-graph/src/main/kotlin/dev/nx/gradle/utils/TaskUtils.kt | Simplify task override naming, always prefix with project name |
packages/gradle/project-graph/src/main/kotlin/dev/nx/gradle/utils/ProjectUtils.kt | Replace cwd parameter with atomized across graph generation |
packages/gradle/project-graph/src/main/kotlin/dev/nx/gradle/utils/CiTargetsUtils.kt | Derive full-qualified test class names for CI targets |
packages/gradle/project-graph/src/main/kotlin/dev/nx/gradle/NxProjectReportTask.kt | Add atomized input property and default convention |
packages/gradle/project-graph/src/main/kotlin/dev/nx/gradle/NxProjectGraphReportPlugin.kt | Remove deprecated cwd property |
packages/gradle/project-graph/build.gradle.kts | Version bumped to 0.0.1-alpha.5 |
packages/gradle/package.json | Added axios dependency |
packages/gradle/batch-runner/src/main/kotlin/dev/nx/gradle/runner/TestListener.kt | Simplify test event handling, use simpleClassName |
packages/gradle/batch-runner/src/main/kotlin/dev/nx/gradle/runner/OutputProcessor.kt | Unified per-task output finalization |
packages/gradle/batch-runner/src/main/kotlin/dev/nx/gradle/runner/GradleRunner.kt | Parallelize builds and tests with coroutines, add --exclude-task |
packages/gradle/batch-runner/src/main/kotlin/dev/nx/gradle/runner/BuildListener.kt | Minor formatting cleanup |
packages/gradle/batch-runner/src/main/kotlin/dev/nx/gradle/data/NxBatchOptions.kt | Added excludeTasks field |
packages/gradle/batch-runner/src/main/kotlin/dev/nx/gradle/data/GradlewTask.kt | Rename testClassName to testName |
packages/gradle/batch-runner/src/main/kotlin/dev/nx/gradle/cli/ArgParser.kt | Parse --excludeTasks argument |
packages/gradle/batch-runner/src/main/kotlin/dev/nx/gradle/NxBatchRunner.kt | Wrap runTasksInParallel in runBlocking |
packages/gradle/batch-runner/build.gradle.kts | Added kotlinx-coroutines-core dependency |
e2e/gradle/src/gradle-import.test.ts | Import and use createFile to write .gitignore |
Comments suppressed due to low confidence (6)
packages/gradle/src/executors/gradle/get-exclude-task.ts:48
- Accessing projectGraph via indexing will always return undefined because dependencies live under
projectGraph.nodes
. Change toprojectGraph.nodes[projectName]?.data?.targets?...
.
const dependsOn = projectGraph[projectName]?.data?.targets?.[targetName]?.dependsOn ?? [];
packages/gradle/src/executors/gradle/get-exclude-task.ts:41
- [nitpick] Neither
getAllDependsOn
norgetExcludeTasks
are covered by unit tests. Add tests to verify recursive dependency collection and exclusion behavior.
export function getAllDependsOn(
packages/gradle/batch-runner/src/main/kotlin/dev/nx/gradle/cli/ArgParser.kt:32
- [nitpick] The CLI flag
--excludeTasks
uses camelCase, but other flags use kebab-case. Consider renaming to--exclude-tasks
for consistency.
val excludeTasks =
argMap["--excludeTasks"]?.split(",")?.map { it.trim() }?.filter { it.isNotEmpty() }
?: emptyList()
packages/gradle/project-graph/src/main/kotlin/dev/nx/gradle/utils/TaskUtils.kt:183
- This always prefixes same-project tasks with
<project>:
and may break existing local dependency names. Restore conditional logic to only prefix whendepProject != taskProject
.
val overriddenTaskName = "${depProject.name}:${taskName}"
packages/gradle/project-graph/build.gradle.kts:13
- [nitpick] The version was downgraded from "0.1.0" to an earlier pre-release. This breaks semantic versioning; consider bumping to a higher patch or prepending a proper pre-release tag.
version = "0.0.1-alpha.5"
packages/gradle/batch-runner/src/main/kotlin/dev/nx/gradle/runner/TestListener.kt:23
- [nitpick] The extra parentheses around the
let
block are unnecessary and make the flow hard to follow. Remove the wrapping(...)
to improve readability.
((event.descriptor as? JvmTestOperationDescriptor)?.className?.substringAfterLast('.')?.let {
@@ -9,7 +9,7 @@ fun createNodeForProject( | |||
project: Project, | |||
targetNameOverrides: Map<String, String>, | |||
workspaceRoot: String, | |||
cwd: String | |||
atomized: Boolean |
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.
[nitpick] The new atomized
parameter lacks documentation. Add a KDoc comment explaining its purpose and the effect on CI target generation.
Copilot uses AI. Check for mistakes.
387cdca
to
a7b74d2
Compare
🐳 We have a release for that!This PR has a release associated with it. You can try it out using this command: npx [email protected] my-workspace Or just copy this version and use it in your own command: 0.0.0-pr-31313-a7b74d2
To request a new release for this pull request, mention someone from the Nx team or the |
a7b74d2
to
1d47572
Compare
🐳 We have a release for that!This PR has a release associated with it. You can try it out using this command: npx [email protected] my-workspace Or just copy this version and use it in your own command: 0.0.0-pr-31313-1d47572
To request a new release for this pull request, mention someone from the Nx team or the |
🐳 We have a release for that!This PR has a release associated with it. You can try it out using this command: npx [email protected] my-workspace Or just copy this version and use it in your own command: 0.0.0-pr-31313-1d47572
To request a new release for this pull request, mention someone from the Nx team or the |
1d47572
to
9688d2f
Compare
🐳 We have a release for that!This PR has a release associated with it. You can try it out using this command: npx [email protected] my-workspace Or just copy this version and use it in your own command: 0.0.0-pr-31313-9688d2f
To request a new release for this pull request, mention someone from the Nx team or the |
9688d2f
to
d972df3
Compare
🐳 We have a release for that!This PR has a release associated with it. You can try it out using this command: npx [email protected] my-workspace Or just copy this version and use it in your own command: 0.0.0-pr-31313-d972df3
To request a new release for this pull request, mention someone from the Nx team or the |
Current Behavior
Expected Behavior
Related Issue(s)
Fixes #