homebeaver commented on code in PR #217:
URL: https://github.com/apache/commons-validator/pull/217#discussion_r1591470221
##########
src/test/java/org/apache/commons/validator/routines/checkdigit/AbstractCheckDigitTest.java:
##########
@@ -154,7 +154,11 @@ public void testCalculateInvalid() {
log.debug(" " + i + " Testing Invalid Check Digit,
Code=[" + code + "]");
}
final String expected = checkDigit(code);
- final String actual =
routine.calculate(removeCheckDigit(code));
+ String codeWithNoCheckDigit = removeCheckDigit(code);
+ if (codeWithNoCheckDigit == null) {
+ throw new CheckDigitException("Invalid Code=[" + code +
"]");
Review Comment:
the default invalid test in `class AbstractCheckDigitTest` is
```
protected String[] invalid = { "12345678A" };
```
method `removeCheckDigit` returns null if `REGEX_VALIDATOR.validate(code)`
== null. This is the case for "12345678A" because this code is invalid due to
its regex.
When using `fail()` as you suggests the test `testCalculateInvalid()` would
fail
but it should `assertTrue(e.getMessage().startsWith("Invalid ") ...` because
"12345678A" IS INVALID!
So we `throw new CheckDigitException("Invalid Code=[" + code + "]");` and
catch it in
```
} catch (final CheckDigitException e) {
// possible failure messages:
// Invalid ISBN Length ...
// Invalid Character[ ...
// Are there any others?
assertTrue(e.getMessage().startsWith("Invalid "), "Invalid
Character[" + i + "]=" + e.getMessage());
```
NB: `testCalculateInvalid()` is not my idea. I needed a while to understand
how it works :-)
Regards
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
