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

[core] Per-rule regex file filter #2956

Open
marquiswang opened this issue Dec 2, 2020 · 7 comments
Open

[core] Per-rule regex file filter #2956

marquiswang opened this issue Dec 2, 2020 · 7 comments
Labels
an:enhancement An improvement on existing features / rules in:ruleset-xml About the ruleset schema/parser

Comments

@marquiswang
Copy link

Is your feature request related to a problem? Please describe.
There are a few rules that I'd like to not run on unit tests. AvoidDuplicateLiterals is the biggest, but some others as well.

Describe the solution you'd like
Would it be possible to add a filter to not exclude files named ".*Test.java", or in test directories, or something, for a specific rule?

Describe alternatives you've considered
For now, I've just disabled the AvoidDuplicateLiterals rule, and suppressed other rules a file at a time.

@marquiswang marquiswang added the an:enhancement An improvement on existing features / rules label Dec 2, 2020
@linusjf
Copy link

linusjf commented Dec 3, 2020

It's not a good idea to add properties about file system or files or wildcards to a specific rule, IMO.
Secondly, the more salient point is why must we relax rules for test code. Shouldn't the same standards or methodologies apply to tests as well?

@oowekyala oowekyala changed the title Per-rule refeg file filter [core] Per-rule regex file filter Dec 3, 2020
@oowekyala oowekyala added the in:ruleset-xml About the ruleset schema/parser label Dec 3, 2020
@marquiswang
Copy link
Author

@Fernal73 There are people who believe that unit tests should emphasize the clarity and descriptiveness of the code in the test over making sure there is no duplication. This is sometimes referred to as "DAMP vs DRY" (see links at bottom)

The simplest case example that comes to mind is with duplicated literals. That the following unit tests for example:

@Test
void monthOfParsedDateIsFirstNumberInUSLocale() {
   assertThat(parseDate("12/3/2020", Locale.US).getMonth()).isEqualTo(DECEMBER);
}

@Test
void monthOfParsedDateIsSecondNumberInUKLocale() {
   assertThat(parseDate("12/3/2020", Locale.UK).getMonth()).isEqualTo(MARCH);
}

In the production code for anything that uses the date parser, you probably don't want any String literals, without even considering duplicated ones. However, with the test code, extracting a constant for the String literal introduces unnecessary coupling between otherwise independent tests, and makes the code harder to read, especially if you have a lot of tests.

@Test
void monthOfParsedDateIsSecondNumberInUKLocale() {
   assertThat(parseDate(DATE, Locale.UK).getMonth()).isEqualTo(MARCH);
}

Of course, you could work around this by changing the string literal to something different in each test, but this is fundamentally a workaround to the fact that you don't care about the duplicated literal.

Other rules you might want to disable for tests are;
ExcessivePublicCount - JUnit 4 unit tests are public methods, but you don't want to put a limit on them.
TooManyMethods - Same reason
SignatureDeclareThrowsException - Looks like PMD already supports disabling this for JUnit
SingularField - Might be necessary for @mock or @captor fields using Mockito
MethodNamingConventions - Since unit test methods are never called, you might have a different convention for their names. I've seen organizations that use "descriptionOfSetup_descriptionOfExpectation" for their unit test method names.

https://enterprisecraftsmanship.com/posts/dry-damp-unit-tests/
https://testing.googleblog.com/2019/12/testing-on-toilet-tests-too-dry-make.html

@linusjf
Copy link

linusjf commented Dec 3, 2020

Thanks for the examples , @marquiswang.

My point was simply that test code tends to become procedural rather than object-oriented, at least, in my case. So I should strive to make it more object-oriented.

As for the literals rule, it kinda sucks. I've had similar issues with the rule in my tests.

Configuring each rule separately for tests can be a huge pain, unless you intend to default the values for the whole ruleset.

@marquiswang
Copy link
Author

Yeah, the problem is that I agree with you for most rules - they should be applied to both production and test code. There are just a few rules that don't make sense.

@linusjf
Copy link

linusjf commented Dec 4, 2020

Yes, it's amazing that you discovered the rules that could do with tweaking for tests. There are probably an issue or two in the PMD database that list these instances. Maybe, they can be merged into this and closed.

@linusjf
Copy link

linusjf commented Dec 4, 2020

This is closeable, IMO.

@oowekyala
Copy link
Member

I think this should be solved by having separate rulesets for test and main sources. You include your main ruleset in the test one and exclude the rules you don't like. You'd have to configure separate executions in maven I think

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
an:enhancement An improvement on existing features / rules in:ruleset-xml About the ruleset schema/parser
Projects
None yet
Development

No branches or pull requests

3 participants