iluwatar / java-design-patterns

Design patterns implemented in Java
https://java-design-patterns.com
Other
89.71k stars 26.55k forks source link

Proper unit tests #293

Closed iluwatar closed 8 years ago

iluwatar commented 8 years ago

Many of the pattern examples have used the convention to just run the main method as a lazy man's unit test but we need to improve this not to be bad example for anyone. The following examples need proper unit tests (I will not make separate issues but rather track this with the following checkboxes):

npathai commented 8 years ago

Totally up for it :+1: It would be good to have meaningful test cases with proper assertions.

fluxw42 commented 8 years ago

Maybe it would be good to remove the AppTest classes, or at least annotate them with @Ignore. These app tests could give a false sense of great test coverage.

Removing these 'test' right now would cause a massive drop in coverage, but adding real junit tests over time should get this number back up, close to the original. You'll see the actual impact of your work when the number goes up, which is nice.

On the other hand, they show that the App at least runs without throwing any errors. What's your opinion on this?

iluwatar commented 8 years ago

I think it is good to have a test that runs the example and fails if there are errors. Therefore I would keep the existing AppTests. Some criticism has been presented that AppTests pollute the build output with the output of the examples. To address that I have been thinking about suppressing the console output when AppTest runs. However, I haven't researched how to do this yet.

fluxw42 commented 8 years ago

@iluwatar Ok, I'll leave the AppTest junit tests in. The console output is not really a problem for me. When a test fails, additional logging could even be useful to find the issue.

fluxw42 commented 8 years ago

The message-channel is marked as untested in this issue, but there is nothing much to test. This pattern only consists of one App class since it's using Apache Camel as message framework. There is an AppTest class, so could this pattern be marked as tested?

iluwatar commented 8 years ago

@fluxw42 you are right. I will tick the checkbox.

Edit: I just noticed that the same applies to publish-subscribe.