Skip to content

Migrate to JUnit5 - Part 1#3945

Merged
oowekyala merged 28 commits into
pmd:pmd/7.0.xfrom
adangel:pmd7-junit5
Jul 16, 2022
Merged

Migrate to JUnit5 - Part 1#3945
oowekyala merged 28 commits into
pmd:pmd/7.0.xfrom
adangel:pmd7-junit5

Conversation

@adangel

@adangel adangel commented Apr 29, 2022

Copy link
Copy Markdown
Member

Describe the PR

Related issues

Ready?

  • Added unit tests for fixed bug/feature
  • Passing all unit tests
  • Complete build ./mvnw clean verify passes (checked automatically by github actions)
  • Added (in-code) documentation (if needed)

@adangel adangel added this to the 7.0.0 milestone Apr 29, 2022
@adangel adangel marked this pull request as draft April 29, 2022 15:24
Comment thread pmd-java/pom.xml
Comment on lines +186 to +190
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter</artifactId>
<scope>test</scope>
</dependency>

@adangel adangel Apr 29, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This dependency needs to be added to each module (only done yet for pmd-apex and pmd-java). Otherwise IntelliJ won't find the tests.

When building with maven, this is not needed, since the surefire-plugin has these dependency explicit added (junit-jupiter-engine), but eventually, we maybe can remove this extra config for surefire.

Comment thread pom.xml
Comment on lines +292 to +296
<!-- JUnit5 Platform Engine for Junit 5 -->
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-engine</artifactId>
<version>${junit.version}</version>

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

By adding this here additionally surefire will now find junit3, junit4, junit5 and kotlin tests.
Ideally we can remove this extra config in the end.

@ghost

ghost commented Apr 29, 2022

Copy link
Copy Markdown
2 Messages
📖 Compared to pmd/7.0.x:
This changeset changes 17 violations,
introduces 18 new violations, 0 new errors and 0 new configuration errors,
removes 18 violations, 0 errors and 0 configuration errors.
Full report
📖 Compared to master:
This changeset changes 57095 violations,
introduces 34423 new violations, 2 new errors and 0 new configuration errors,
removes 138133 violations, 25 errors and 6 configuration errors.
Full report
Compared to pmd/7.0.x:
This changeset changes 27 violations,
introduces 20 new violations, 0 new errors and 0 new configuration errors,
removes 26 violations, 0 errors and 0 configuration errors.
Full report
Compared to master:
This changeset changes 57096 violations,
introduces 34416 new violations, 2 new errors and 0 new configuration errors,
removes 138132 violations, 25 errors and 6 configuration errors.
Full report
No regression tested rules have been changed.

Generated by 🚫 Danger

@adangel adangel linked an issue Apr 29, 2022 that may be closed by this pull request
3 tasks
@adangel

adangel commented Jun 21, 2022

Copy link
Copy Markdown
Member Author

Maybe user Assertions already in master and use static imports consistently

@adangel adangel changed the title Migrate to JUnit5 Migrate to JUnit5 - Part 1 Jun 30, 2022
adangel added 2 commits June 30, 2022 09:14
Update wiremock dependency
- Use static imports for assertions
- Use package private tests and classes when possible
@adangel adangel mentioned this pull request Jun 30, 2022
6 tasks
@adangel adangel marked this pull request as ready for review June 30, 2022 08:46

@adangel adangel left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think, this part 1 is now ready. All tests in pmd-core are migrated.
I'll continue with the other modules in a follow-up PR - this is one is already big...

Maybe user Assertions already in master and use static imports consistently
I didn't change anything on master yet. Not sure, we should do it. Assertions is junit5 only, which means, we would need to add the junit5 api as a dependency.
However, I changed the style to be consistent (only in pmd-core for now): Now we always use static imports for the assertions/assumptions.

Comment on lines +127 to +129
SystemLambda.restoreSystemProperties(() -> {
runPmdSuccessfully("--no-cache", "--dir", srcDir, "--rulesets", DUMMY_RULESET, "--report-file", reportFile, "--debug");
});

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This in an example of how to use SystemLambda, which replaces junit4 system rules

Comment on lines +206 to +208
String log = SystemLambda.tapSystemErrAndOut(() -> {
runPmd(StatusCode.VIOLATIONS_FOUND, "--no-cache", "--dir", srcDir, "--rulesets", "dummy-basic");
});

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is an example to use SystemLambda in order to capture sysout/syserr (we used previously SystemOutRule/SystemErrRule)

Comment on lines +40 to +41
@TempDir
private Path tempDir;

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is a core JUnit5 extension which replaces the TemporaryFolder junit4 rule.
-> https://junit.org/junit5/docs/current/user-guide/#writing-tests-built-in-extensions-TempDirectory

Instead of declaring a field, it can also be used as a test method parameter.

Comment on lines +88 to +90
@ParameterizedTest
@MethodSource("childrenIndexes")
void testRemoveChildOfRootNode(final int childIndex) {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Example for a parameterized test, which is now a native JUnit5 feature
-> https://junit.org/junit5/docs/current/user-guide/#writing-tests-parameterized-tests

@adangel adangel removed a link to an issue Jun 30, 2022
3 tasks
@oowekyala oowekyala self-assigned this Jul 15, 2022
@oowekyala oowekyala merged commit 942c8f0 into pmd:pmd/7.0.x Jul 16, 2022
@adangel adangel deleted the pmd7-junit5 branch July 18, 2022 19:24
@adangel adangel mentioned this pull request Oct 3, 2022
3 tasks
@adangel adangel mentioned this pull request Oct 13, 2022
13 tasks
@adangel adangel added the in:testing About tests of pmd, eg the module pmd-lang-test or pmd-test [test] label Jan 12, 2023
@adangel adangel mentioned this pull request Jan 23, 2023
55 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

in:testing About tests of pmd, eg the module pmd-lang-test or pmd-test [test]

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants