Hi,
As I see rule java:S1874 detect using of deprecated code only in production code.
I think it will be also useful if I can detect the same in my test classes.
Hi Slawomir,
At the moment, it is a hard-coded list in the SonarJava plugin which rules are applied to the main code and which ones are applied to the test code. It might be a good idea to rethink that assignment or make it even configurable for the users. Thanks for your feedback! I have created a ticket (SONARJAVA-4905) about your suggestion.
Best,
Marco
2 Likes
Thanks for creating issue.
Maybe instead of making possibility to configure assignment of rule to code scope (it looks as a bigger change), you can simply add new rule with test code scope.
1 Like
In my opinion, it would be interesting that Sonar scans tests!
As of today, Sonar can scan tests, but it will do it as if tests were production code. However, test code has its own specificity which don’t quite match production code.
For example, it is excellent practice to organize tests using the 3A rule - Arrange, Act, Assert. This rule is specific only to tests and is meaningless for production code.
Another example would be in the usage of tests framework: using .isNull() instead of .isEqualTo(null). And so on, and so on…
The topic of this discussion is yet another example of how useful test quality scans would be.
Bottom line is: tests are a world within itself, with its best practices and rules. And I would love to see it covered by SonarQube!
3 Likes
I agree that there are certain aspects to tests which are specific - such as the 3A rule you mention and indeed simple checks like “are there actually any tests in this file”? or “are there any asserts in this test?”.
Unfortunately for other languages where there is a differentiation between test and non-test code, SonarQube have gone the route of having only a handful of rules for tests and completely disregard all of the other rules - this is wrong IMO as tests are just as important as production code - so why would we not hold them to a higher standard?
Arguably in some ways more important - if you take the view that the tests are “living documentation” for how the production code is expected to behave, if you cannot understand the tests then what hope do you have of being able to figure out what they or the production code are doing and if this is expected or not?
I would like to see, across all languages that Sonar supports, the ability to apply a significant subset of the production code rules to test code AND supplement with additional rules that make sense only to tests, such as the ones outlined earlier.
Why only most and not all productio code rules? Well applying the rule about using a testable DateTime provider makes no sense in a test that is itself testing a testable DateTime provider for example 
And there are other rules you might want relaxed such as ones about hard coded passwords where you are checking your password strength algorithm etc.
In our organisation for the languages we support, we disable the detection of test code so that all of our code is considered “production” - this has the disadvantage of not being able to detect test-specific things (like no tests) and counts against our LOC license, but the benefits of being able to ensure the quality of the tests generally outweigh the disadvantages.
2 Likes
I agree with you about the test rules to be checked. Not sure I got the part about the DateTime though, but I guess we are roughly on the same lines.
I’m quite new to this forum, but quite willing to contribute. Is it possible to open a new thread about this subject?
Not sure I got the part about the DateTime though
Certainly in dotnet builds, we often get a Sonar rule telling us to use a testable DateTime provider - which makes sense in production code, but not in test code where you are for example testing your testable DateTime provider, or indeed simply where your test does not need the complexity of same.
I’m quite new to this forum, but quite willing to contribute. Is it possible to open a new thread about this subject?
You could start a new thread for sure - anyone on the forums can start threads.
1 Like
Hi all,
Thanks for raising this topic! I agree with the points shared about test code.
As a first step, we are enabling some Java rules that make sense in this context.
I’m happy to report that java:S1874 is included in the first batch of 20 rules now enabled for test code by default. This has been released in SonarQube 10.6 and is already available in SonarCloud. It will be coming in the following version of SonarLint too.
We are actively working on this, and we’ll continue to add more. Stay tuned!
1 Like
Thanks, Can you list which of 20 rules was changed … or point to release notes which describe it?
1 Like
Is there any reason not to enable most, if not all rules for test code? Perhaps with a way to disable the small number that are likely inappropriate for tests?
Also do you plan to roll this out to more languages like c# and python?
1 Like
Hi all,
For reference here is the announcement and the complete list of rules as of today:
Our approach has been to try the rules internally with many projects to ensure they are not noisy and to enable only the ones that bring enough value. This also helps us notice potential false positives and improve the rules.
We are continuing this effort, so expect more rules in the future.
1 Like
Our approach has been to try the rules internally with many projects to ensure they are not noisy and to enable only the ones that bring enough value
Shouldn’t it be up to the customers which rules bring them value?
Ideally IMO all rules can be applied to tests but we have the power to disable certain rules for tests which is distinct from disabling them for everything or only for non test code
Hi Tony,
I think that’s fair.
Also keep in mind that we have hundreds of thousands of users, and having each of them take the time to review 700+ rules seems like a big ask.
Reviewing them gives us the chance to improve them for test code, and skip the rules that would be very noisy for all users. We’re hoping we can enable most of them pregressively, so stay tuned.
If you want to try all the rules for test code today, you can configure your test code as the main code see the docs.
Also keep in mind that we have hundreds of thousands of users, and having each of them take the time to review 700+ rules seems like a big ask.
Sure so the default behaviour would be as it is today - tests are only covered by the existing test-only rules, but you have the option to run most/all rules against tests as well as other code, in a configurable way.
You guys reviewing rules and deciding what is/is not noisy from your perspective does not mean that your hundreds of thousands of users would agree with whatever you decide - please keep the power in the hands of the users and do not be proscriptive about this.
You deciding that rule FOO is not that noisy in your cases says nothing about the experience of others where they could become a complete nightmare if enforce.
In our projects we do run all the rules against test and non-test code but this means that the rules that apply to tests only do not get run at all.
1 Like
Hi Tony,
We know the current scope feature can be improved, especially for the reasons you share: it doesn’t give enough control. We’re looking into this, and sharing your views is very helpful.
In our projects we do run all the rules against test and non-test code but this means that the rules that apply to tests only do not get run at all.
Thanks a lot for this feedback!
This topic was automatically closed 7 days after the last reply. New replies are no longer allowed.