Skip to content
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

Add archunit-junit5 pom aggregator #275

Merged
merged 6 commits into from
Dec 20, 2019
Merged

Conversation

eddumelendez
Copy link
Contributor

Resolves #272

@ghost
Copy link

ghost commented Dec 9, 2019

DeepCode's analysis on #8a7e45 found:

  • 0 critical issues. ⚠️ 0 warnings and 0 minor issues. ✔️ 0 issues were fixed.

💬 This comment has been generated by the DeepCode bot, installed by the owner of the repository. The DeepCode bot protects your repository by detecting and commenting on security vulnerabilities or other critical issues.


☺️ If you want to provide feedback on our bot, here is how to contact us.

Copy link
Contributor

@rweisleder rweisleder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍
Thanks, @eddumelendez!

@codecholeric
Copy link
Collaborator

Thanks a lot for looking into this 😃 It seems to work, but I have two things to consider

a) I did get some exception during archunit-junit5:shadowJar, which actually did not let the build fail or broke the result, but we should still fix this (I did not see it in the console output of the Travis build though)
b) a bigger thing I wonder is, if we really want to create all those artifacts. I.e. now we create an empty JAR, and empty javadoc and an empty sources JAR. I've noticed that the JUnit team does the same, but I wonder why we would not simply use a POM of type pom? Similar to https://repo1.maven.org/maven2/com/google/guava/guava-parent/28.1-jre/. Shouldn't a simple POM without any artifacts attached to it also do the trick?

@eddumelendez
Copy link
Contributor Author

@codecholeric the exception is displayed building against java 11 at least in my machine. You can see my findings below:

a) The new commit resolves the issue building against java 11. I couldn't upgrade to 4.0.4 due to one issue which was supposed to be fixed in that version but it wasn't until version 5.x. Upgrading to 5.x requires gradle version. I think there is more to see regarding this dependency upgrade. You can see more about shadow plugin building against java 11 in the release notes

b) That would be something that gradle understand and can resolve the dependencies but maven users will need to specify an additional line type meanwhile gradle users will not. Keeping the jar will benefit both.

@codecholeric
Copy link
Collaborator

@eddumelendez ah, cool, thanks a lot for looking into this 😄

a) Yes, we need to upgrade Gradle ☹️ Luckily there is light at the horizon thanks to @aaschmid (see #192)
b) Okay, I didn't consider the implication with the additional type 😉 Since the aggregator POM strives only to make including ArchUnit as simple as possible, I agree! It's probably best to produce the triplet of JAR, sources JAR and javadoc JAR then...

eddumelendez and others added 6 commits December 20, 2019 14:03
Issue: TNG#272

Signed-off-by: Eddú Meléndez <eddu.melendez@gmail.com>
Signed-off-by: Eddú Meléndez <eddu.melendez@gmail.com>
Signed-off-by: Eddú Meléndez <eddu.melendez@gmail.com>
Now that the Shadow Plugin is working after the upgrade, it started to repackage dependencies into a fat JAR, making archunit-junit5.jar quite big with clutter. We can exclude all files, then the Manifest Plugin will generate the correct manifest and the rest will be empty.

Signed-off-by: Peter Gafert <peter.gafert@tngtech.com>
Now that we have the new aggregator POM, we should also let the Maven integration test run against this dependency for extra test coverage. Also adjusted the Surefire configuration to current best practices from the JUnit 5 user guide (the approach to add the engine as dependency to the Surefire plugin was given up a while ago. In particular since the Surefire plugin took over the provider from the JUnit team, that part does not work anymore. I.e. while Maven now natively supports JUnit 5, it only does so, if the test engine is in the project classpath, not if it is in the Surefire plugin classpath.)

Signed-off-by: Peter Gafert <peter.gafert@tngtech.com>
@codecholeric
Copy link
Collaborator

I have looked over it, looks good now 😄
I noticed that the upgraded Shadow plugin now was "too good" 😉 (started to aggregate some 3rd party stuff into archunit-junit5.jar) I've adjusted the config to simply exclude everything. Seems reasonable now, three empty JARs with just a Manifest and a POM with the correct dependencies.
I've adjusted the Maven integration test to now test JUnit 5 with the new aggregator dependency and it seems to work.
So thank you so much for your support!! 😃 Gonna merge it now...

@codecholeric codecholeric merged commit 9361475 into TNG:master Dec 20, 2019
codecholeric added a commit that referenced this pull request Feb 21, 2021
Add archunit-junit5 pom aggregator
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide an Aggregator POM for archunit-junit5
3 participants