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

Set Junit5 as default test engine #1187

Merged

Conversation

abelsromero
Copy link
Member

@abelsromero abelsromero commented Apr 22, 2023

Kind of change

  • Bug fix
  • New non-breaking feature
  • New breaking feature
  • Documentation update
  • Build improvement

Description

What is the goal of this pull request?

Make JUnit5 de default testing framework.
Not possible in Wilddly integration test which requires heavy Arquillian changes and the use of an Arquiliar RC version (arquillian/arquillian-core#137 (comment)).

How does it achieve that?

  • Sets JUnit5 as the default test engine.

  • Rewrites polluted tests "category" into a JUnit 5 "tag". The Tag is applied via new annotation @Polluted which I personally thing it's nicer that plain using the tag.

  • Only minimal changes are done, test that can still work in "junit 4 mode" using junit-vintage-engine still use. But at the end only core project requires it, some minimal rewrites allowed to remove it from documentation, cli and distribution modules.

  • Removed use of ClasspathResource Test Rule which is not compatible, currently just replaced with use of ClasspathHelper + initializations in each test, I may refactor better when I get some extra time. I want to reduce boilerplate with a junit5 extension in another PR (see checklist).

  • Restrict codenarc configuration to only required modules

  • Other changes

    • Bump codenarc to latest -> Using the codenarc configuration did not override the Groovy version in use, I applied it via constraint. It can be validated by listing dependencies for codenarc configuration (./gradlew :module:dependencies).
    • Bump Groovy to v3.0.x
    • Add documentation reports to GH update upload
    • Fix build and upstream tests reports overriding each other when uploading

Are there any alternative ways to implement this?

Maybe go for "big bang" but I'd rather not do that. We can get something working, merge, and then migrate over time, next way we are removing the use of deprecated methods from tests when we need to review them.

Are there any implications of this pull request? Anything a user must know?

No

Issue

Related to #1186, we'll see how far we can get to completion in this one PR. 🤞

Release notes

Please add a corresponding entry to the file CHANGELOG.adoc

Added a note to the change just in case someone follows the repo and wants to see what's going on, I will update with the next PR when all is done and we can consider the migration completed.

@abelsromero abelsromero force-pushed the issue-1186-migrate-to-junit5 branch 2 times, most recently from 2d1fa7d to 099baec Compare April 22, 2023 22:49
@abelsromero
Copy link
Member Author

Windows failures...

@abelsromero
Copy link
Member Author

It seems Windows has some issues cleaning test directories, it's related to junit-team/junit5#2811. Right now I am considering not deleting temp directories 🤔
The OS will eventually do that.

@abelsromero abelsromero force-pushed the issue-1186-migrate-to-junit5 branch 4 times, most recently from 387a451 to 9538408 Compare April 23, 2023 18:57
@abelsromero abelsromero marked this pull request as ready for review April 23, 2023 19:01
@abelsromero
Copy link
Member Author

Most of the changes are boilerplate, but there are still a lot. No hurry.

* Remove use of Arquillian Sputnik components: not compatible
* Restrict codenarc configuration to modules using Spock
* Bump codenarc to latest
* Bump Groovy to v3.0.x
* Fix deprecated 'defaultVersion' use of jruby Gradle plugin
* CI: Add documentation reports upload in CI
* CI: Fix upload reports naming overlap
* Disable cleanup for @tempdir to fix Windows tests failing unable to delete
@robertpanzer
Copy link
Member

Let's get modern! 🚀

P.S.: Sorry for that stupid question, where can I find the logs that seem to be uploaded from the Github action? Sorry if I sound stupid, but I've never used that feature yet :)

@robertpanzer robertpanzer merged commit c1d4e4a into asciidoctor:main Apr 24, 2023
@abelsromero
Copy link
Member Author

P.S.: Sorry for that stupid question

Not at all, in fact imo the location should be improved. Just open an execution detail sand scroll to botom https://github.com/asciidoctor/asciidoctorj/actions/runs/4788558210 to the "Artifacts" section.

We could have a custom job that aggregates them all to have a single zip with some bash, but for now, I am going for a quick one. Downside is yaml duplication.
The env mapping in the config is necessary because you cannot use matrix variables directly 🤷

@abelsromero abelsromero deleted the issue-1186-migrate-to-junit5 branch May 2, 2023 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants