You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@grails.apache.org by "matrei (via GitHub)" <gi...@apache.org> on 2026/07/30 08:04:50 UTC
[PR] build: improve project dependency substitution logic in functional te… [grails-core]
matrei opened a new pull request, #16069:
URL: https://github.com/apache/grails-core/pull/16069
(no comment)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve project dependency substitution logic [grails-core]
Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty merged PR #16069:
URL: https://github.com/apache/grails-core/pull/16069
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve project dependency substitution logic [grails-core]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #16069:
URL: https://github.com/apache/grails-core/pull/16069#discussion_r3683083348
##########
gradle/functional-test-config.gradle:
##########
@@ -17,49 +17,53 @@
* under the License.
*/
-rootProject.subprojects
+def substitutableProjects = rootProject.subprojects
.findAll { !(it.name in testProjects) && !(it.name in docProjects) && !(it.name in cliProjects) }
- .each { project.evaluationDependsOn(it.path) }
+ .collect {
+ project.evaluationDependsOn(it.path)
+ return [
+ project: it,
+ artifactId: it.findProperty('pomArtifactId') ?: it.name,
+ cliArtifactId: it.findProperty('cliArtifactId')
+ ]
+ }
Review Comment:
`substitutableProjects` is no longer a collection of projects; it’s a collection of maps with metadata. Consider renaming it (e.g., `substitutions`, `substitutableProjectMetadata`) and renaming the map key `project` (e.g., `targetProject`) to avoid confusion with Gradle’s `project` object in this script.
##########
gradle/functional-test-config.gradle:
##########
@@ -17,49 +17,53 @@
* under the License.
*/
-rootProject.subprojects
+def substitutableProjects = rootProject.subprojects
.findAll { !(it.name in testProjects) && !(it.name in docProjects) && !(it.name in cliProjects) }
- .each { project.evaluationDependsOn(it.path) }
+ .collect {
+ project.evaluationDependsOn(it.path)
+ return [
+ project: it,
+ artifactId: it.findProperty('pomArtifactId') ?: it.name,
+ cliArtifactId: it.findProperty('cliArtifactId')
+ ]
+ }
configurations.configureEach {
resolutionStrategy.dependencySubstitution {
// Test projects will often include dependencies from local projects. This will ensure any dependencies
// included will be substituted with the local projects in this repository instead of pulling upstream.
- for (def possibleProject : rootProject.subprojects) {
- if (!(possibleProject.name in testProjects) && !(possibleProject.name in docProjects) && !(possibleProject.name in cliProjects)) {
- def artifactId = possibleProject.findProperty('pomArtifactId') ?: possibleProject.name
- def substitutedArtifact = "$possibleProject.group:$artifactId"
- //TODO: This does not handle libraries that are both test fixtures & a libraries like grails-data-mongodb,
- // see grails-test-examples-mongodb-base, & grails-test-examples-mongodb-hibernate5 for project() workaround
- if (possibleProject.name == 'grails-bom') {
- substitute module(substitutedArtifact) using platform(project(':grails-bom'))
- }
- else if(possibleProject.name == 'grails-geb') {
- def selector = it.variant(module(substitutedArtifact)) { VariantSelectionDetails details ->
- details.capabilities { ModuleDependencyCapabilitiesHandler handler ->
- handler.requireCapability("${substitutedArtifact}-test-fixtures" as String)
- }
- }
-
- def replacement = it.variant(project(":$possibleProject.name")) { v ->
- v.capabilities { it.requireCapability("${substitutedArtifact}-test-fixtures") }
+ for (def substitution : substitutableProjects) {
+ def possibleProject = substitution.project
+ def substitutedArtifact = "$possibleProject.group:${substitution.artifactId}"
+ //TODO: This does not handle libraries that are both test fixtures & a libraries like grails-data-mongodb,
Review Comment:
Grammar issue in the TODO comment: 'a libraries' should be singular.
##########
gradle/functional-test-config.gradle:
##########
@@ -17,49 +17,53 @@
* under the License.
*/
-rootProject.subprojects
+def substitutableProjects = rootProject.subprojects
.findAll { !(it.name in testProjects) && !(it.name in docProjects) && !(it.name in cliProjects) }
- .each { project.evaluationDependsOn(it.path) }
+ .collect {
+ project.evaluationDependsOn(it.path)
+ return [
+ project: it,
+ artifactId: it.findProperty('pomArtifactId') ?: it.name,
+ cliArtifactId: it.findProperty('cliArtifactId')
+ ]
+ }
Review Comment:
The explicit `return` inside the `collect { ... }` closure is unnecessary in Groovy (the last expression is returned). Dropping it simplifies the closure and makes the intent a bit clearer.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve project dependency substitution logic [grails-core]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #16069:
URL: https://github.com/apache/grails-core/pull/16069#issuecomment-5131298627
## [Codecov](https://app.codecov.io/gh/apache/grails-core/pull/16069?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 0.0000%. Comparing base ([`301b40c`](https://app.codecov.io/gh/apache/grails-core/commit/301b40c20acdb8650ea36cf685fbd54a6341bccb?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`cc471e4`](https://app.codecov.io/gh/apache/grails-core/commit/cc471e4f48c63e5ab1ab53356e364b4b3aea68b6?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
[](https://app.codecov.io/gh/apache/grails-core/pull/16069?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## 8.0.x #16069 +/- ##
=================================================
- Coverage 53.1945% 0 -53.1945%
=================================================
Files 1980 0 -1980
Lines 92519 0 -92519
Branches 16075 0 -16075
=================================================
- Hits 49215 0 -49215
+ Misses 36076 0 -36076
+ Partials 7228 0 -7228
```
[see 1980 files with indirect coverage changes](https://app.codecov.io/gh/apache/grails-core/pull/16069/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve project dependency substitution logic [grails-core]
Posted by "matrei (via GitHub)" <gi...@apache.org>.
matrei commented on code in PR #16069:
URL: https://github.com/apache/grails-core/pull/16069#discussion_r3699687154
##########
gradle/functional-test-config.gradle:
##########
@@ -17,49 +17,53 @@
* under the License.
*/
-rootProject.subprojects
+def substitutableProjects = rootProject.subprojects
.findAll { !(it.name in testProjects) && !(it.name in docProjects) && !(it.name in cliProjects) }
- .each { project.evaluationDependsOn(it.path) }
+ .collect {
+ project.evaluationDependsOn(it.path)
+ return [
+ project: it,
+ artifactId: it.findProperty('pomArtifactId') ?: it.name,
+ cliArtifactId: it.findProperty('cliArtifactId')
Review Comment:
Comment added.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve project dependency substitution logic [grails-core]
Posted by "matrei (via GitHub)" <gi...@apache.org>.
matrei commented on code in PR #16069:
URL: https://github.com/apache/grails-core/pull/16069#discussion_r3699677644
##########
gradle/functional-test-config.gradle:
##########
@@ -17,49 +17,53 @@
* under the License.
*/
-rootProject.subprojects
+def substitutableProjects = rootProject.subprojects
.findAll { !(it.name in testProjects) && !(it.name in docProjects) && !(it.name in cliProjects) }
- .each { project.evaluationDependsOn(it.path) }
+ .collect {
+ project.evaluationDependsOn(it.path)
+ return [
+ project: it,
+ artifactId: it.findProperty('pomArtifactId') ?: it.name,
+ cliArtifactId: it.findProperty('cliArtifactId')
+ ]
+ }
configurations.configureEach {
resolutionStrategy.dependencySubstitution {
// Test projects will often include dependencies from local projects. This will ensure any dependencies
// included will be substituted with the local projects in this repository instead of pulling upstream.
- for (def possibleProject : rootProject.subprojects) {
- if (!(possibleProject.name in testProjects) && !(possibleProject.name in docProjects) && !(possibleProject.name in cliProjects)) {
- def artifactId = possibleProject.findProperty('pomArtifactId') ?: possibleProject.name
- def substitutedArtifact = "$possibleProject.group:$artifactId"
- //TODO: This does not handle libraries that are both test fixtures & a libraries like grails-data-mongodb,
- // see grails-test-examples-mongodb-base, & grails-test-examples-mongodb-hibernate5 for project() workaround
- if (possibleProject.name == 'grails-bom') {
- substitute module(substitutedArtifact) using platform(project(':grails-bom'))
- }
- else if(possibleProject.name == 'grails-geb') {
- def selector = it.variant(module(substitutedArtifact)) { VariantSelectionDetails details ->
- details.capabilities { ModuleDependencyCapabilitiesHandler handler ->
- handler.requireCapability("${substitutedArtifact}-test-fixtures" as String)
- }
- }
-
- def replacement = it.variant(project(":$possibleProject.name")) { v ->
- v.capabilities { it.requireCapability("${substitutedArtifact}-test-fixtures") }
+ for (def substitution : substitutableProjects) {
+ def possibleProject = substitution.project
+ def substitutedArtifact = "$possibleProject.group:${substitution.artifactId}"
+ //TODO: This does not handle libraries that are both test fixtures & a libraries like grails-data-mongodb,
Review Comment:
TODO comment was not changed in this PR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve project dependency substitution logic [grails-core]
Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on code in PR #16069:
URL: https://github.com/apache/grails-core/pull/16069#discussion_r3683252757
##########
gradle/functional-test-config.gradle:
##########
@@ -17,49 +17,53 @@
* under the License.
*/
-rootProject.subprojects
+def substitutableProjects = rootProject.subprojects
.findAll { !(it.name in testProjects) && !(it.name in docProjects) && !(it.name in cliProjects) }
- .each { project.evaluationDependsOn(it.path) }
+ .collect {
Review Comment:
Optional follow-up, not a blocker. This snapshot is rebuilt independently by each of the 104 test projects that apply this script, so the `findAll` plus 134 `evaluationDependsOn`/`findProperty` calls all run 104 times over.
Computing it once and sharing it (keeping a cheap per-project `evaluationDependsOn` loop so evaluation ordering is unchanged) measured a further **~10% off configuration time, 3.69s → 3.32s**, with identical resolution output.
For completeness, I also tried precomputing the `"$group:$artifactId"` coordinate strings into the snapshot so the loop body does no GString work at all — that was only 3.69s → 3.53s, near noise, so it is not worth doing for performance alone. The bulk of the win is already in this PR.
##########
gradle/functional-test-config.gradle:
##########
@@ -17,49 +17,53 @@
* under the License.
*/
-rootProject.subprojects
+def substitutableProjects = rootProject.subprojects
.findAll { !(it.name in testProjects) && !(it.name in docProjects) && !(it.name in cliProjects) }
- .each { project.evaluationDependsOn(it.path) }
+ .collect {
+ project.evaluationDependsOn(it.path)
+ return [
+ project: it,
+ artifactId: it.findProperty('pomArtifactId') ?: it.name,
+ cliArtifactId: it.findProperty('cliArtifactId')
Review Comment:
This moves the `cliArtifactId` read from lazy (per configuration, when the substitution rules are registered) to eager (immediately after `evaluationDependsOn`). That matters because `cliArtifactId` is only set inside an `afterEvaluate` in `GrailsCliArtifactGradlePlugin`, which `gradle/cli-companion-bom-constraints.gradle` documents as the reason it uses a lazy `withDependencies` window.
It is correct as written — `evaluationDependsOn` flushes the target's `afterEvaluate` before returning, and I confirmed the snapshot matches the late value for all 134 substitutable projects across all 104 applying test projects with zero mismatches.
The only thing I would add is a short comment recording that invariant. The snapshot is now silently order-dependent: if `cliArtifactId` ever moves to a hook later than `afterEvaluate`, or gets set cross-project, the cli substitutions would quietly stop being registered rather than fail loudly, which is an unpleasant failure mode to debug.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve project dependency substitution logic [grails-core]
Posted by "matrei (via GitHub)" <gi...@apache.org>.
matrei commented on code in PR #16069:
URL: https://github.com/apache/grails-core/pull/16069#discussion_r3699684977
##########
gradle/functional-test-config.gradle:
##########
@@ -17,49 +17,53 @@
* under the License.
*/
-rootProject.subprojects
+def substitutableProjects = rootProject.subprojects
.findAll { !(it.name in testProjects) && !(it.name in docProjects) && !(it.name in cliProjects) }
- .each { project.evaluationDependsOn(it.path) }
+ .collect {
+ project.evaluationDependsOn(it.path)
+ return [
+ project: it,
+ artifactId: it.findProperty('pomArtifactId') ?: it.name,
+ cliArtifactId: it.findProperty('cliArtifactId')
+ ]
+ }
Review Comment:
I think it helps readability here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve project dependency substitution logic [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #16069:
URL: https://github.com/apache/grails-core/pull/16069#issuecomment-5148311557
## ✅ All tests passed ✅
🏷️ Commit: d4945dba74c1f55d115e56390a12e1ac4aaa6c35
▶️ Tests: 39497 executed
⚪️ Checks: 60/60 completed
---
_Learn more about TestLens at [testlens.app](https://testlens.app)._
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve project dependency substitution logic [grails-core]
Posted by "matrei (via GitHub)" <gi...@apache.org>.
matrei commented on code in PR #16069:
URL: https://github.com/apache/grails-core/pull/16069#discussion_r3699745664
##########
gradle/functional-test-config.gradle:
##########
@@ -17,49 +17,53 @@
* under the License.
*/
-rootProject.subprojects
+def substitutableProjects = rootProject.subprojects
.findAll { !(it.name in testProjects) && !(it.name in docProjects) && !(it.name in cliProjects) }
- .each { project.evaluationDependsOn(it.path) }
+ .collect {
Review Comment:
Cached the snapshot so gain the additional 10%
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve project dependency substitution logic [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #16069:
URL: https://github.com/apache/grails-core/pull/16069#issuecomment-5130158316
## ✅ All tests passed ✅
🏷️ Commit: cc471e4f48c63e5ab1ab53356e364b4b3aea68b6
▶️ Tests: 50978 executed
⚪️ Checks: 60/60 completed
---
_Learn more about TestLens at [testlens.app](https://testlens.app)._
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org