You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2020/10/14 21:53:21 UTC
[GitHub] [commons-validator] nhojpatrick opened a new pull request #45: JUnit assertThrows
nhojpatrick opened a new pull request #45:
URL: https://github.com/apache/commons-validator/pull/45
Code could be cleaner if it could be Java 1.8.
I could set `<maven.compiler.testSource>1.8</maven.compiler.testSource>`... thoughts???
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-validator] garydgregory commented on a change in pull request #45: WIP DO NOT MERGE YET JUnit assertThrows
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #45:
URL: https://github.com/apache/commons-validator/pull/45#discussion_r505572171
##########
File path: src/test/java/org/apache/commons/validator/routines/DomainValidatorStartupTest.java
##########
@@ -35,24 +39,52 @@
@RunWith( ClassLoaderPerTestRunner.class )
public class DomainValidatorStartupTest {
- @Test(expected = IllegalArgumentException.class)
+ @Test
public void testUpdateBaseArrayCC() {
- DomainValidator.updateTLDOverride(ArrayType.COUNTRY_CODE_RO, new String[]{"com"});
+ // FIXME Simplification once upgraded to Java 1.8
+ final ThrowingRunnable testMethod = new ThrowingRunnable() {
+ public void run() {
+ DomainValidator.updateTLDOverride(ArrayType.COUNTRY_CODE_RO, new String[]{"com"});
+ }
+ };
+ final IllegalArgumentException thrown = assertThrows(IllegalArgumentException.class, testMethod);
+ assertThat(thrown.getMessage(), is(equalTo("Cannot update the table: COUNTRY_CODE_RO")));
Review comment:
Same comment as in the Apache Commons Collections PR. Let's skip asserting the exception messages.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-validator] nhojpatrick commented on a change in pull request #45: JUnit assertThrows
Posted by GitBox <gi...@apache.org>.
nhojpatrick commented on a change in pull request #45:
URL: https://github.com/apache/commons-validator/pull/45#discussion_r506942291
##########
File path: src/test/java/org/apache/commons/validator/routines/DomainValidatorStartupTest.java
##########
@@ -35,24 +39,52 @@
@RunWith( ClassLoaderPerTestRunner.class )
public class DomainValidatorStartupTest {
- @Test(expected = IllegalArgumentException.class)
+ @Test
public void testUpdateBaseArrayCC() {
- DomainValidator.updateTLDOverride(ArrayType.COUNTRY_CODE_RO, new String[]{"com"});
+ // FIXME Simplification once upgraded to Java 1.8
+ final ThrowingRunnable testMethod = new ThrowingRunnable() {
+ public void run() {
+ DomainValidator.updateTLDOverride(ArrayType.COUNTRY_CODE_RO, new String[]{"com"});
+ }
+ };
+ final IllegalArgumentException thrown = assertThrows(IllegalArgumentException.class, testMethod);
+ assertThat(thrown.getMessage(), is(equalTo("Cannot update the table: COUNTRY_CODE_RO")));
Review comment:
Why skip? It's a common conversation I always keep having... I just wondered your rational...
My argument to alway checking the exception message is how do you know the test is failing as you are expecting it to fail?
You write a test to check X, and it throws Y exception. If Z might also causes Y to be thrown, so how do you know your testing X? My view is if you skip checking the exception message then you might as well just delete the test.
So Digester BeanPropertySetterRuleTestCase.testSetUnknownProperty according to the error messages and comments should produce an NoSuchMethodException, but currently is throws org.xml.sax.SAXException due to a code path meaning any non expected Exception is ignored.
So this test is to check COUNTRY_CODE_RO, but if the code behind the test changes and IllegalArgumentException is still thrown, how do you know it's the correct exception for what you have setup?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-validator] garydgregory commented on pull request #45: JUnit assertThrows
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #45:
URL: https://github.com/apache/commons-validator/pull/45#issuecomment-708712400
I am ok with Java 8 but others might not be. May you please bring this up on the dev ML? It would be better if using Java 8 would bring benefits to the main code.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-validator] nhojpatrick commented on pull request #45: JUnit assertThrows
Posted by GitBox <gi...@apache.org>.
nhojpatrick commented on pull request #45:
URL: https://github.com/apache/commons-validator/pull/45#issuecomment-709241654
@garydgregory have continued assertThrows email about maven.compiler.testSource.
About the jdk 1.7 build failure, i'm not sure exactly what the issue is as from what i'm aware i've not touched anything to do with `japicmp/maven/JApiCmpMojo`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org