RepoSense
RepoSense copied to clipboard
RepoConfigurationTest: Simplify code in `repoConfig_ignoresStandaloneConfig_success` test case
The code snippet below shows the repoConfig_ignoresStandaloneConfig_success
test case.
In lines 153 - 157, we parse some input to create a list of RepoConfiguration
objects.
https://github.com/reposense/RepoSense/blob/a2adb592431b2ee59fb3d22a3f6c8ab703870de9/src/test/java/reposense/model/RepoConfigurationTest.java#L130-L164
The lines 153 - 157 can be replaced with the line below:
List<RepoConfiguration> actualConfigs = RepoSense.getRepoConfigurations((ConfigCliArguments) cliArguments);
It seems that the lines 153 - 157 are duplicating the same work done in the getRepoConfigurations
method of the RepoSense
class.
Please let me know if I'm misunderstanding something here.
Yes, it is duplicating some parts of the method. However, I believe it is necessary.
- This is the unit test for
RepoConfiguration
. Apart from the method directly from the class itself, we should use the minimum outside method. Even if we have to use, we should use the lower abstraction one. So, the change of other classes and other methods will infect this unit test minimally. A unit test is meant to test the behavior of this specific class. - Looking to the method itself, also it is longer than just use
RepoSense::getRepoConfigurations
, however, it only uses highly related one and the class method.RepoSense::getRepoConfigurations
is an outsider method with high abstraction, we certainly do not want the change of this method will affect the unit test of other models.
Thanks for the response @fzdy1914.
It makes sense that it may not be a good idea to use RepoSense::getRepoConfigurations
in a test because future changes to this method would affect the test.
In that case, perhaps we should edit the repoConfig_ignoresStandaloneConfigInCli_success
test case, since it uses the RepoSense::getRepoConfigurations
method ? (see line 179 in snippet below)
https://github.com/reposense/RepoSense/blob/a2adb592431b2ee59fb3d22a3f6c8ab703870de9/src/test/java/reposense/model/RepoConfigurationTest.java#L167-L186
@anubh-v True, it is better not to use this method. When I wrote this method previously, I should forget to take it into account.
However, it may be an overkill to create a separate PR for just fixing this issue. Maybe you can consider sneaking in the code when toughing this file in other related PR.