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

CommandLineParser tests should only test one feature per test case #150

Open
cmnbroad opened this issue Jan 14, 2019 · 0 comments
Open

CommandLineParser tests should only test one feature per test case #150

cmnbroad opened this issue Jan 14, 2019 · 0 comments

Comments

@cmnbroad
Copy link
Collaborator

cmnbroad commented Jan 14, 2019

There are a lot of tests that are ganged up to test multiple features per case, but this can result in confusing and misleading results due to serial mutations of the test data. For example, the testNullOptionals method has code that purports to test that a required arg can't be set to NULL:

    //verify that required arg can't be set to null
    args[2] = "TRUTHINESS=null";
    final LegacyCommandLineArgumentParser clp2 = new LegacyCommandLineArgumentParser(fownl);
    Assert.assertFalse(clp2.parseArguments(System.err, args));

This test passes because the call to parseArguments does return false, but the false value results not from the attempt to set TRUTHINESS to null, but from the fact that an earlier call in the method to parseArguments sets the value of FROBNICATION_THRESHOLD to null (previous value 20). Because FROBNICATION_THRESHOLD has an initial value, the first parseArguments call treats it as optional. But once the state of FROBNICATION_THRESHOLD is set to null, the parser subsequently views it as required, and the next call returns false because of the attempt to set it to null. So the TRUTHINESS part of the test is never even triggered.

All of the tests should be separated to ensure the starting state is consistent.

@cmnbroad cmnbroad changed the title CommandLineArgumentParser tests should only test one feature per test case CommandLineParser tests should only test one feature per test case Jan 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant