-
Notifications
You must be signed in to change notification settings - Fork 244
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
Update unit test doc #5856
Update unit test doc #5856
Conversation
Signed-off-by: Chong Gao <res_life@163.com>
build |
Made an example,
|
tests/README.md
Outdated
The `tests` module depends on the `aggregator` module which is shading the dependencies. | ||
Should first `mvn install` the aggregator jar to local Maven repository due to a limitation of the shading system. | ||
The steps to run the unit tests: |
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.
@tgravescs does this wording make sense?
The `tests` module depends on the `aggregator` module which is shading the dependencies. | |
Should first `mvn install` the aggregator jar to local Maven repository due to a limitation of the shading system. | |
The steps to run the unit tests: | |
The `tests` module depends on the `aggregator` module which shades dependencies. When running the | |
tests, make sure to do an install via `mvn install` for the aggregator jar to the local maven | |
repository. | |
The steps to run the unit tests: |
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.
its close, we might add "When running the tests via 'mvn test', make...
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.
Updating the suggestion:
The `tests` module depends on the `aggregator` module which is shading the dependencies. | |
Should first `mvn install` the aggregator jar to local Maven repository due to a limitation of the shading system. | |
The steps to run the unit tests: | |
The `tests` module depends on the `aggregator` module which shades dependencies. When running the | |
tests via `mvn test`, make sure to do an install via `mvn install` for the aggregator jar to the | |
local maven repository. | |
The steps to run the unit tests: |
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.
Done
build |
If we end up merely documenting the work around we should file another bug to actually make Ideally we should make test depend on dist, make sure that dist mimics the default maven phases correctly and customize test class path of the test module if necessary using |
Thanks, @gerashegalov
If Already had an issue to try to depend on |
Thanks for the reminder about #3192 @res-life. In addition, another obstacle to making the So there is no way to make the |
Closes #5467
Problem
case 1
The aggregator is a shading module, there are no classes under the
aggregator/target/Sparkxxx/classes/
path.So Maven tries to find the aggregator jar from local or remote repositories.
case 2
By default, Maven tries to find an aggregator jar from local or remote repositories.
Changes
Due to the limitation of Maven, should first install the aggregator jar to the local repository.
Only need to update the doc.
Other context
The aggregator jar is not published to Maven Central.
Signed-off-by: Chong Gao res_life@163.com