Stop using the USER_DIR test variable
#10890
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #10890 +/- ##
=========================================
Coverage 57.57% 57.57%
Complexity 1702 1702
=========================================
Files 346 346
Lines 12829 12829
Branches 1212 1212
=========================================
Hits 7386 7386
Misses 4978 4978
Partials 465 465
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| private fun Carthage.resolveDependencies(definitionFile: File) = | ||
| resolveDependencies(USER_DIR, definitionFile, Excludes.EMPTY, AnalyzerConfiguration(), emptyMap()) | ||
| resolveDependencies(definitionFile.parentFile, definitionFile, Excludes.EMPTY, AnalyzerConfiguration(), emptyMap()) |
There was a problem hiding this comment.
This functions seems a bit redundant with resolveSingleProject(). Would it make sense to drop it in favor of resolveSingleProject() maybe in a preceeding commit?
|
|
||
| val result = GradleInspectorFactory.create(javaVersion = "17") | ||
| .resolveDependencies(USER_DIR, listOf(definitionFile), Excludes.EMPTY, AnalyzerConfiguration(), emptyMap()) | ||
| val result = GradleInspectorFactory.create(javaVersion = "17").resolveDependencies( |
There was a problem hiding this comment.
Also here resolveSingleProject() ? ..and below.
edit: I just noticed the limitation of resolveSingleProject() to return just a single ProjectAnalyzerResult. However, we could have a similar helper function which does not assume single result, but which helps removing code redundancy in an analog way? Something like resolveSingleDefinitionFile()
There was a problem hiding this comment.
I just noticed the limitation of
resolveSingleProject()to return just a singleProjectAnalyzerResult.
That's exactly the reason why I didn't do a migration here yet. I agree it's probably meaningful to do, but I'd like to consider this out of scope for this PR.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
plugins/package-managers/carthage/src/test/kotlin/CarthageTest.kt
Dismissed
Show dismissed
Hide dismissed
sschuberth
force-pushed
the
no-user-dir
branch
from
e5999f5 to
5f8575a
Compare
Signed-off-by: Sebastian Schuberth <[email protected]>
sschuberth
force-pushed
the
no-user-dir
branch
7 times, most recently
from
775d91f to
f40df96
Compare
Pull request was converted to draft
sschuberth
force-pushed
the
no-user-dir
branch
from
f40df96 to
a9aab2d
Compare
`USER_DIR` is the current user / working directory which does not necessarily have anything in common with the definition files. Use a more realistic analyzer root that is a parent of the definition files instead. For `CargoSubcrateFunTest` this actually fixes the `PackageLinkage` as it is determined based in the `analyzerRoot`. This corrects "Crate::lib:0.1.0" to appear in the list of packages. For various other tests, this simplifies the project ID / paths of expected results as the project root now equals the `analyzerRoot`. Signed-off-by: Sebastian Schuberth <[email protected]>
Signed-off-by: Sebastian Schuberth <[email protected]>
Reuse this function to align with other tests. Signed-off-by: Sebastian Schuberth <[email protected]>
sschuberth
force-pushed
the
no-user-dir
branch
from
a9aab2d to
c3a0438
Compare
| import org.ossreviewtoolkit.model.config.ScopeExcludeReason | ||
| import org.ossreviewtoolkit.plugins.api.PluginConfig | ||
| import org.ossreviewtoolkit.utils.test.USER_DIR | ||
| import org.ossreviewtoolkit.utils.common.getCommonParentFile |
There was a problem hiding this comment.
commit-msg: Is there another reason for no more using USER_DIR which is not mentioned?
Please have a look at the individual commit messages for the details.