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

Naming convention for test classes #552

Closed
Bukama opened this issue Dec 4, 2021 · 4 comments
Closed

Naming convention for test classes #552

Bukama opened this issue Dec 4, 2021 · 4 comments

Comments

@Bukama
Copy link
Member

Bukama commented Dec 4, 2021

During our preparations for Pioneer 1.0 release we discussed about naming conventions for almost all of our classes etc (see #215 ). But we didn't talked about naming test classes.

@Marcono1234 revealed in #543 that currently test classes must (^1) end with Tests (plural) to be picked up by the correspondending Gradle action. This issue extracts his question if we shoul also allow the singular variant Test which is frequently used by programmers.

The maven surefire and failsafe plugin pick up both Test and Tests classes by default.

What confuses me is: Are classes which ends on Test are really not picked up? We have:
the AbstractEntryBasedExtensionTest class and I'm quite sure it is picked up (as it sometimes fails). This test is renamed in the PR too to match the naming conventions

@Bukama
Copy link
Member Author

Bukama commented Dec 7, 2021

I would add the singular Test too, as I sometimes write singular and sometimes plural, depending on the number of tests in a class.

@Marcono1234
Copy link
Contributor

What confuses me is: Are classes which ends on Test are really not picked up? We have:
the AbstractEntryBasedExtensionTest class and I'm quite sure it is picked up (as it sometimes fails).

At least for me Gradle does not seem to execute the test class when its name ends with Test. Note however that IDEs might perform test class detection in a different way and therefore can end up executing that test class.

@beatngu13
Copy link
Member

What confuses me is: Are classes which ends on Test are really not picked up? We have:
the AbstractEntryBasedExtensionTest class and I'm quite sure it is picked up (as it sometimes fails).

@Bukama the test isn't executed via Gradle:

https://sonarcloud.io/component_measures?id=junit-pioneer_junit-pioneer&metric=tests&selected=junit-pioneer_junit-pioneer%3Asrc%2Ftest%2Fjava%2Forg%2Fjunitpioneer%2Fjupiter

As you can see, AbstractEntryBasedExtensionTest.java contains no unit tests according to Sonar (marked with "–"). You can also verify it locally when you activate debug logging in Gradle.

@nipafx
Copy link
Member

nipafx commented Dec 9, 2021

I'd rather live with the rare case that a class called ...Tests only has a single test than the inevitable mixture of ...Test and ...Tests classes. My vote is to stick to ...Tests and document that in the contribution guide.

What about inner classes that we execute manually, though? They're usually (always) called ...TestCase(s), sometimes singular, sometimes plural. This should probably be unified as well. Is there a semantic argument to be made why a single ...TestCase contains multiple test methods? Otherwise, I'd vote to rename them all to ...TestCases.

(Tip for the implementor: I couldn't immediately find these classes in the open type dialog and instead of thinking about the problem or looking for some check box to tick, I did a full text search for "TestCase(s).class".)

jbduncan added a commit to jbduncan/junit-pioneer that referenced this issue Dec 9, 2021
jbduncan added a commit to jbduncan/junit-pioneer that referenced this issue May 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Exploring Io
Awaiting triage
Development

No branches or pull requests

4 participants