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

Test Smell: testing private methods is not a good practice in testing #3873

Closed
TestSmell opened this issue Aug 11, 2022 · 0 comments
Closed

Comments

@TestSmell
Copy link

TestSmell commented Aug 11, 2022

Hi!
I notice that in your test code, you try to use reflection to assess and test the private method.
For example,
the test methods named gatewayWithDestExpression() in JmsOutboundGatewayParserTests uses the reflection method to test a private method named determineReplyDestination(Message<?> message, Session session) in JmsOutboundGateway.

But, the problem of testing private methods has a wide discussion in Stack Overflow, and the majority of answers think it is a bad test practice. Because it may cause many problems:

  1. many answers believe that if you need to test private methods, it means your production code is not well designed, i.e., design smell.
  2. testing private methods breaks the encapsulation.
  3. Many answers mention that private methods are invoked by public ones, so test methods just need to test public methods. Otherwise, if a private method needs to be tested separately, it may be dead code.
  4. Testing implementation details (private methods) directly would make the tests harder to maintain and the code-under-test more difficult to refactor.

Although several methods are used to access private methods (e.g., reflection methods, mock tests, changing the modifier to make it visible), refactoring the corresponding code is regarded as the best practice to eliminate this bad test practice.

So, we kindly suggest you refactor the corresponding source code instead of using the reflection method to test the private method.

@TestSmell TestSmell added status: waiting-for-triage The issue need to be evaluated and its future decided type: enhancement labels Aug 11, 2022
@artembilan artembilan added in: jms and removed status: waiting-for-triage The issue need to be evaluated and its future decided labels Aug 11, 2022
@artembilan artembilan added this to the 6.0.0-M5 milestone Aug 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants