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

[pekko-testkit-typed] Support for JUnit5 in testkit #751

Merged
merged 13 commits into from
Nov 10, 2023

Conversation

thmue
Copy link
Contributor

@thmue thmue commented Oct 19, 2023

Due to some issues with the scala setup i created a new PR, which is based on the current main branch.

This PR replaces #445

The extension is activated through an annotation:
@ExtendWith(TestKitJUnit5Extension.class)
on the class level.

Additionally, a testKit with a @Junit5TestKit annotation needs to be created.
A builder pattern is used to construct the Testkit instance.

@JUnit5TestKit
public ActorTestKit testKit = new JUnit5TestKitBuilder().build();

The extension itself uses the JUnit5 AfterAllCallback to shutdown the testkit after all tests are executed.

Copy link
Contributor

@mdedetrich mdedetrich left a comment

Choose a reason for hiding this comment

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

While its true that we shouldn't rename the Junit4 class names, we should actually fix them and have the old class name as an alias that is deprecated (so we don't break binary compatibility).

There is no reason not to improve the current state of things and consistency with current incorrectly named code isn't a compelling argument if you can fix the original issue without breaking users so I would recommend the following

  • Rename the classes back to JUnit4/JUnit5 so its actually consistent with how JUnit is named.
  • Create deprecated Junit4/Junit5 classes that extend JUnit4/JUnit5 so it doesn't break binary compatibility. These can then be dropped in Pekko 2.0.x

@thmue
Copy link
Contributor Author

thmue commented Oct 20, 2023

ok i changed all Junit5 names/labels to JUnit5. I suggest to rename the other Junit4 labels / class names in a different PR.

@mdedetrich
Copy link
Contributor

@thmue There are some compile errors for Scala 3

Copy link
Contributor

@pjfanning pjfanning left a comment

Choose a reason for hiding this comment

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

lgtm - @mdedetrich wdyt? - it may be worth merging as is and worrying about prospective changes to the junit4 existing code later

@pjfanning
Copy link
Contributor

@mdedetrich is it possible to merge this?

@pjfanning pjfanning added this to the 1.1.0 milestone Nov 10, 2023
@mdedetrich
Copy link
Contributor

I would like to add the aliases as I mentioned before, if it's tok much work then I can do it in a follow up PR

@pjfanning
Copy link
Contributor

I would like to add the aliases as I mentioned before, if it's tok much work then I can do it in a follow up PR

I think that would be better in a separate PR

Copy link
Contributor

@mdedetrich mdedetrich left a comment

Choose a reason for hiding this comment

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

lgtm, lets do alias changes in separate PR

@pjfanning pjfanning merged commit 153b9c2 into apache:main Nov 10, 2023
17 of 18 checks passed
@thmue thmue deleted the junit5-ext branch November 24, 2023 21:26
@aseychell
Copy link

I would like to write tests with Junit5 and it seems this PR was merged 8 months ago but this is still not available on any stable release. Are there any plans to make this available please?

@raboof
Copy link
Member

raboof commented Jul 5, 2024

Good question! I agree we should be working towards an 1.1.0 release. I think the best way to help with that is to look at the issues and PRs linked to the 1.1.0-M2 milestone and see if there's anything in there that you'd feel comfortable picking up.

@pjfanning
Copy link
Contributor

I would like to write tests with Junit5 and it seems this PR was merged 8 months ago but this is still not available on any stable release. Are there any plans to make this available please?

If you don't want to use 1.1.0-M1 (and it is not a full release), you could copy the 4 small classes from this PR into your code base and use them Pekko 1.0.x.

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.

5 participants