Isolate scoverage modules from their parent modules #3118
Merged
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
romain-gilles-ultra
changed the title
lefou
reviewed
Apr 11, 2024
There was a problem hiding this comment.
I think with the changes from #3090 and #3064, the classpath part of interest is outer.localRunClasspath. If we override the runClasspath and remove outer.localRunClasspath from it and add outer.scoverage.localRunClasspath, we should be done, without needing to introduce the shadow test module.
Also, this PR changes various other places unrelated to the essential feature. It would be good to keep the changeset minimal.
|
I removed the unrelated changes I will open an other PR for those one if you are ok with that |
Co-authored-by: Tobias Roeser <le.petit.fou@web.de>
romain-gilles-ultra
force-pushed
the
isolate-scoverage
branch
from
6dc6647
to
974c029
Compare
|
@romain-gilles-ultra Could you please update the PR description to reflect the actual change? |
Done |
Quafadas
pushed a commit
to Quafadas/mill
that referenced
this pull request
Apr 15, 2024
## Context I encountered an issue with the way scoverage plugin alters the dependency tree of the test module by changing its module dependency from the `outer` module to the `outer.scoverage` module. This shadow modification of the dependency tree produces a side effect that impacts the IDEA configuration generation. When the IDEA configuration generation resolves the dependencies instead of using the outer module as a dependency for the test project it depends on the `outer.scoverage` module which should not be generated as it does not exist factually but more virtually. ## Solution Instead of modifying the dependency tree at compile-time I change it at runtime by removing the `outer.localRunClasspath()` from the `test.runClasspath()` and appending, instead, the `outer.scoverage.localRunClasspath()` This resolves the issue of modifying the dependency tree but introduces one more compilation as when the `outer.localRunClasspath()` is resolved it will force the compilation of the outer module. In the previous version only the `outer.scoverage` was compiled and not both of them. Pull request: com-lihaoyi#3118
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.
Context
I encountered an issue with the way scoverage plugin alters the dependency tree of the test module by changing its module dependency from the
outermodule to theouter.scoveragemodule.This shadow modification of the dependency tree produces a side effect that impacts the IDEA configuration generation.
When the IDEA configuration generation resolves the dependencies instead of using the outer module as a dependency for the test project it depends on the
outer.scoveragemodule which should not be generated as it does not exist factually but more virtually.Solution
Instead of modifying the dependency tree at compile-time I change it at runtime by removing the
outer.localRunClasspath()from thetest.runClasspath()and appending, instead, theouter.scoverage.localRunClasspath()This resolves the issue of modifying the dependency tree but introduces one more compilation as when the
outer.localRunClasspath()is resolved it will force the compilation of the outer module. In the previous version only theouter.scoveragewas compiled and not both of them.