GitLab

Another once-over, and more problems found:

• testExceptionsViolatedInRandomConstructor and testExceptionsViolatedInEvilConstructor do not match the spec.

assertThrows(IllegalStateException.class, () -> Reflection.newInstance(c, -1, 3));

should throw an IllegalArgumentxception by the spec, since wordLength < 1. A more appropriate additional test would be

assertThrows(IllegalStateException.class, () -> Reflection.newInstance(c, Integer.MAX_VALUE, 3));

• The spec currently expects IllegalStateExceptions due to an empty wordlist to be thrown in the makeGuess method. The tests expect them to be thrown in the constructor. Either the spec or the tests should be corrected for consistency.

• testMakeGuessExceptionsInRandom is not mentioned in the spec, instead stated as an assumption. I suggest changing the part about assuming valid inputs to something about making sure to validate their inputs and throw an appropriate exception if the input is not valid.

• testMakeGuessExceptionsInRandom does not test what it is supposed to test. I don't know what Reflection.getMethod() tries to pass in for the constructor, but it isn't a valid argument for the attributes, so it triggers the IllegalAttributeException of the constructor. Not sure what best practice is in this case - @blank?

• The trace tests appear to be correct at this point, no further changes needed here.

changed the description

@blank @jkarras

testExceptionsViolatedInRandomConstructor and testExceptionsViolatedInEvilConstructor do not match the spec. -- agreed, fixed.

The spec currently expects IllegalStateExceptions due to an empty wordlist to be thrown in the makeGuess method. The tests expect them to be thrown in the constructor. Either the spec or the tests should be corrected for consistency. -- agreed @jkarras please fix

testMakeGuessExceptionsInRandom is not mentioned in the spec, instead stated as an assumption. I suggest changing the part about assuming valid inputs to something about making sure to validate their inputs and throw an appropriate exception if the input is not valid. -- agreed @jkarras please fix

testMakeGuessExceptionsInRandom does not test what it is supposed to test. I don't know what Reflection.getMethod() tries to pass in for the constructor, but it isn't a valid argument for the attributes, so it triggers the IllegalAttributeException of the constructor. Not sure what best practice is in this case -- this should be fixed now, but I didn't have time right now to actually test it; so, @eordentl please let me know

The spec should be fixed now!

@blank

Worked out how to fix the testMakeGuessExceptionsInRandom and testMakeGuessExceptionsInEvil - the IllegalArgumentException is being thrown because the target object is in the wrong place and because the guess needs to be cast back to a char. Correct code is

IntStream.range(0, 20).forEach(i -> assertThrows(IllegalArgumentException.class, () -> m.invoke(chooser, (char) ('a' - (i + 1)))));
IntStream.range(0, 20).forEach(i -> assertThrows(IllegalArgumentException.class, () -> m.invoke(chooser, (char) ('z' + (i + 1)))));

Also:

• Test doesn't check for IllegalStateException due to an empty wordlist. In the constructor exception tests, add

assertThrows(IllegalStateException.class, () -> Reflection.newInstance(c, Integer.MAX_VALUE, 3));

• I'm really not sure how I missed this, but GuesserTests has the tests for the choosers, and ChooserTests has the tests for the guesser.

@jkarras

@jkarras are these all fixed now? if so, please close this issue

closed