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

Replace JUnit4 by JUnit5 #1192

Merged

Conversation

abelsromero
Copy link
Member

@abelsromero abelsromero commented Apr 25, 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?

Complete #1186. JUnit5 is the default now (only used in 'asciidoctorj-wildfly-integration-test' because of Arquillian).
And we have a new JUnit5 tools.
The Arquillian extension is left untouched.

How does it achieve that?

Replace Junit4 by Junit5

* Implement new Junit5 extensions:
   @AsciidoctorInstance & @ClasspathResource annotations
   available to inject Asciidoctor instances and resources into tests.
* Update all modules to use JUnit5 and the new extensions
  'asciidoctorj-wildfly-integration-test' remains on JUnit4 for
  Arquillian compatibility.
Other changes:
* Remove some uses of deprecated APIs
* Fix typo in sample doc
* Replace use of IOUtils.readFull(File) by Files.readString(Path)
  where possible to remove third-party dependencies.

Are there any alternative ways to implement this?

Not on the base implementation, but the naming of the JUnit5 extensions annotations is open for debate 😅 (naming is hard).

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

No.

Issue

This completely closes the issue 🎉

Fixes #1186

Release notes

Please add a corresponding entry to the file CHANGELOG.adoc

Already added.

@abelsromero
Copy link
Member Author

I just realized we publish the asciidoctorj-test-support artifact.
Right now I have a JUnit5 extension that offers:

  • @ClassResource will inject a shared Asciidoctor instance into a test field.
  • @TestMethodResource will inject an new Asciidoctor for each test method.
  • @ClasspathResource("test.txt") will inject a file from classpath. Can be used as filed or test parameters.

With that, we cover our needs, but maybe the ClassResource and TestMethodResource nomenclature is not very clear for users. Any suggestion, I was thinking calling it @AsciidoctorInstance with a property that accepts an enum to be shared or not 🤔

@abelsromero
Copy link
Member Author

Made some more progress 🎉 the arquillian extension is not used but we can keep it, and only the wildfly-integration-test module uses junit 4 now.

I am also thinking renaming asciidoctorj-test-support to asciidoctorj-junit-extenasion, that would make it's intent clear, and that way it can awaken interest from users.

@abelsromero abelsromero force-pushed the issue-1186-remove-junit4 branch 3 times, most recently from 56f6e36 to 0c51f2a Compare April 29, 2023 18:45
* Implement new Junit5 extensions:
   @AsciidoctorInstance & @ClasspathResource annotations
   available to inject Asciidoctor instances and resources into tests.
* Update all modules to use JUnit5 and the new extensions
  'asciidoctorj-wildfly-integration-test' remains on JUnit4 for
  Arquillian compatibility.
Other changes:
* Remove some uses of deprecated APIs
* Fix typo in sample doc
* Replace use of IOUtils.readFull(File) by Files.readString(Path)
  where possible to remove third-party dependencies.

Closes asciidoctor#1186
@abelsromero abelsromero mentioned this pull request Apr 29, 2023
2 tasks
@abelsromero abelsromero changed the title issue-1186-remove-junit4 Replace JUnit4 by JUnit5 Apr 29, 2023
@abelsromero abelsromero marked this pull request as ready for review April 29, 2023 18:51
@abelsromero
Copy link
Member Author

Ready 🎉

@robertpanzer
Copy link
Member

Thank you for this! This is awesome! 🙌

@robertpanzer robertpanzer merged commit e131d22 into asciidoctor:main May 1, 2023
@abelsromero abelsromero deleted the issue-1186-remove-junit4 branch May 2, 2023 17:59
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.

Migrate to JUnit5
2 participants