-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Fix package path computation in ClasspathScanner #2531
Conversation
Prior to this commit no trailing `/` character was appended to the computed package path. Now, except for the default package `""`, a `/` is appended to package path. This leads to corrected and documented behavior even if two modules start with the same name elements. Fixes #2500
Codecov Report
@@ Coverage Diff @@
## main #2531 +/- ##
============================================
+ Coverage 90.56% 90.57% +0.01%
- Complexity 4683 4684 +1
============================================
Files 407 407
Lines 11568 11571 +3
Branches 923 923
============================================
+ Hits 10476 10480 +4
Misses 814 814
+ Partials 278 277 -1
Continue to review full report at Codecov.
|
} | ||
{ | ||
var classes = classpathScanner.scanForClassesInPackage("com.greetings.test", allClasses); | ||
var classNames = classes.stream().map(Class::getName).collect(Collectors.toList()); |
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.
nit: I'd use a static import for toList()
.
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.
Let's wait until we switched to Java 16 -- and replace all those with Stream::toList
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.
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.
Go for it! 👍
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.
How was the platform-tests/src/test/resources/com.greetings.test@0-ea.jar
file created; what does it contain; and can we reproduce/modify it?
Good questions. I'll replace this JAR file and the related Btw. do we know how to reproduce/modify |
40b8377
to
a7ec3a4
Compare
Prior to this commit no trailing `/` character was appended to the computed package path. Now, except for the default package `""`, a `/` is appended to package path. This leads to corrected and documented behavior even if two modules start with the same name elements. Fixes #2500
a7ec3a4
to
6cf0011
Compare
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.
👍
I don't know. Sounds like it may be problematic. 🤷♂️ |
Thanks! 👍 |
Fix `getRootUrisForPackage()` in class `ClasspathScanner` by looking for two "wordings" of a package name. For example, package `foo.bar` is expanded to `foo/bar` and `foo/bar/`. The latter allows find package `foo.bar` in a module called `foo.bar`, and not `foo`. This commit also ensures, that there's no regression (compared to #2531) by launching test runs using the standalone artifact with `--select-package` and `--scan-class-path`. Fixes #2500 Fixes #2600 Fixes #2612
Fix `getRootUrisForPackage()` in class `ClasspathScanner` by looking for two "wordings" of a package name. For example, package `foo.bar` is expanded to `foo/bar` and `foo/bar/`. The latter allows find package `foo.bar` in a module called `foo.bar`, and not `foo`. This commit also ensures, that there's no regression (compared to #2531) by launching test runs using the standalone artifact with `--select-package` and `--scan-class-path`. Fixes #2500 Fixes #2600 Fixes #2612
Fix `getRootUrisForPackage()` in class `ClasspathScanner` by looking for two "wordings" of a package name. For example, package `foo.bar` is expanded to `foo/bar` and `foo/bar/`. The latter allows find package `foo.bar` in a module called `foo.bar`, and not `foo`. This commit also ensures, that there's no regression (compared to junit-team#2531) by launching test runs using the standalone artifact with `--select-package` and `--scan-class-path`. Fixes junit-team#2500 Fixes junit-team#2600 Fixes junit-team#2612
Overview
Prior to this commit no trailing
/
character was appended to the computed package path.Now, except for the default package
""
, a/
is appended to package path. This leads to corrected and documented behavior even if two modules start with the same name elements.Fixes #2500
I hereby agree to the terms of the JUnit Contributor License Agreement.
Definition of Done
@API
annotations