[jira] [Commented] (BCEL-343) Improve JUnit assertions
[ https://issues.apache.org/jira/browse/BCEL-343?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17209580#comment-17209580 ] Allon Mureinik commented on BCEL-343: - Verified, closing. > Improve JUnit assertions > > > Key: BCEL-343 > URL: https://issues.apache.org/jira/browse/BCEL-343 > Project: Commons BCEL > Issue Type: Task > Components: Build > Reporter: Allon Mureinik >Priority: Major > Fix For: 6.5.1 > > > Followup to BCEL-342, based on the discussion in the PR > ([https://github.com/apache/commons-bcel/pull/68#issuecomment-703671353)] - > now that the project used JUnit Jupiter, it's worth checking if the > assertions can be improved to use String suppliers instead of string methods > and improve performance. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Closed] (BCEL-343) Improve JUnit assertions
[ https://issues.apache.org/jira/browse/BCEL-343?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Allon Mureinik closed BCEL-343. --- > Improve JUnit assertions > > > Key: BCEL-343 > URL: https://issues.apache.org/jira/browse/BCEL-343 > Project: Commons BCEL > Issue Type: Task > Components: Build > Reporter: Allon Mureinik >Priority: Major > Fix For: 6.5.1 > > > Followup to BCEL-342, based on the discussion in the PR > ([https://github.com/apache/commons-bcel/pull/68#issuecomment-703671353)] - > now that the project used JUnit Jupiter, it's worth checking if the > assertions can be improved to use String suppliers instead of string methods > and improve performance. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (BCEL-343) Improve assertions
[ https://issues.apache.org/jira/browse/BCEL-343?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17209463#comment-17209463 ] Allon Mureinik commented on BCEL-343: - PR posted here: [https://github.com/apache/commons-bcel/pull/69] > Improve assertions > -- > > Key: BCEL-343 > URL: https://issues.apache.org/jira/browse/BCEL-343 > Project: Commons BCEL > Issue Type: Task > Components: Build > Reporter: Allon Mureinik >Priority: Major > > Followup to BCEL-342, based on the discussion in the PR > ([https://github.com/apache/commons-bcel/pull/68#issuecomment-703671353)] - > now that the project used JUnit Jupiter, it's worth checking if the > assertions can be improved to use String suppliers instead of string methods > and improve performance. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (BCEL-343) Improve assertions
[ https://issues.apache.org/jira/browse/BCEL-343?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17208341#comment-17208341 ] Allon Mureinik commented on BCEL-343: - I'm looking into this. Not quite sure what the rules about the assignment of issues are, and aren't sure if the "Assignee" field is reserved for maintainers, but I'm on it. > Improve assertions > -- > > Key: BCEL-343 > URL: https://issues.apache.org/jira/browse/BCEL-343 > Project: Commons BCEL > Issue Type: Task > Components: Build >Reporter: Allon Mureinik >Priority: Major > > Followup to BCEL-342, based on the discussion in the PR > ([https://github.com/apache/commons-bcel/pull/68#issuecomment-703671353)] - > now that the project used JUnit Jupiter, it's worth checking if the > assertions can be improved to use String suppliers instead of string methods > and improve performance. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Created] (BCEL-343) Improve assertions
Allon Mureinik created BCEL-343: --- Summary: Improve assertions Key: BCEL-343 URL: https://issues.apache.org/jira/browse/BCEL-343 Project: Commons BCEL Issue Type: Task Components: Build Reporter: Allon Mureinik Followup to BCEL-342, based on the discussion in the PR ([https://github.com/apache/commons-bcel/pull/68#issuecomment-703671353)] - now that the project used JUnit Jupiter, it's worth checking if the assertions can be improved to use String suppliers instead of string methods and improve performance. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Closed] (BCEL-342) Upgrade testing framework to JUnit Jupiter
[ https://issues.apache.org/jira/browse/BCEL-342?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Allon Mureinik closed BCEL-342. --- Verified the code was indeed merged to master, closing. > Upgrade testing framework to JUnit Jupiter > -- > > Key: BCEL-342 > URL: https://issues.apache.org/jira/browse/BCEL-342 > Project: Commons BCEL > Issue Type: Task > Components: Build > Reporter: Allon Mureinik >Priority: Major > Fix For: 6.5.1 > > > The Commons BCEL project uses JUnit 4, with several tests still written in > the style of JUnit 3 (test classes extending {{junit.framework.TestCase}}, > etc.) > Converting the test suite to use the modern JUnit 5 Jupiter will make the > tests easier to maintain and update. Additionally, as JUnit Jupiter is > rapidly becoming the industry standard (if it isn't already), migrating the > test suite to use it will make it easier to onboard new contributors. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (BCEL-342) Upgrade testing framework to JUnit Jupiter
[ https://issues.apache.org/jira/browse/BCEL-342?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17206686#comment-17206686 ] Allon Mureinik commented on BCEL-342: - I pushed the following PR to GitHub to address this suggestion: https://github.com/apache/commons-bcel/pull/68 > Upgrade testing framework to JUnit Jupiter > -- > > Key: BCEL-342 > URL: https://issues.apache.org/jira/browse/BCEL-342 > Project: Commons BCEL > Issue Type: Task > Components: Build > Reporter: Allon Mureinik >Priority: Major > > The Commons BCEL project uses JUnit 4, with several tests still written in > the style of JUnit 3 (test classes extending {{junit.framework.TestCase}}, > etc.) > Converting the test suite to use the modern JUnit 5 Jupiter will make the > tests easier to maintain and update. Additionally, as JUnit Jupiter is > rapidly becoming the industry standard (if it isn't already), migrating the > test suite to use it will make it easier to onboard new contributors. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Created] (BCEL-342) Upgrade testing framework to JUnit Jupiter
Allon Mureinik created BCEL-342: --- Summary: Upgrade testing framework to JUnit Jupiter Key: BCEL-342 URL: https://issues.apache.org/jira/browse/BCEL-342 Project: Commons BCEL Issue Type: Task Components: Build Reporter: Allon Mureinik The Commons BCEL project uses JUnit 4, with several tests still written in the style of JUnit 3 (test classes extending {{junit.framework.TestCase}}, etc.) Converting the test suite to use the modern JUnit 5 Jupiter will make the tests easier to maintain and update. Additionally, as JUnit Jupiter is rapidly becoming the industry standard (if it isn't already), migrating the test suite to use it will make it easier to onboard new contributors. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (FILEUPLOAD-302) Upgrade test framework to JUnit Jupiter
[ https://issues.apache.org/jira/browse/FILEUPLOAD-302?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16945589#comment-16945589 ] Allon Mureinik commented on FILEUPLOAD-302: --- A PR with this change was posted at [https://github.com/apache/commons-fileupload/pull/23] > Upgrade test framework to JUnit Jupiter > --- > > Key: FILEUPLOAD-302 > URL: https://issues.apache.org/jira/browse/FILEUPLOAD-302 > Project: Commons FileUpload > Issue Type: Task >Reporter: Allon Mureinik >Priority: Minor > Time Spent: 10m > Remaining Estimate: 0h > > Commons FileUpload uses JUnti 4.12, which is a tad upgraded. > Migrating to the modern JUnit Jupiter may help ease new contributors' > participation. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Created] (FILEUPLOAD-302) Upgrade test framework to JUnit Jupiter
Allon Mureinik created FILEUPLOAD-302: - Summary: Upgrade test framework to JUnit Jupiter Key: FILEUPLOAD-302 URL: https://issues.apache.org/jira/browse/FILEUPLOAD-302 Project: Commons FileUpload Issue Type: Task Reporter: Allon Mureinik Commons FileUpload uses JUnti 4.12, which is a tad upgraded. Migrating to the modern JUnit Jupiter may help ease new contributors' participation. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (CSV-252) Upgrade test framework to JUnit Jupiter
[ https://issues.apache.org/jira/browse/CSV-252?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16945127#comment-16945127 ] Allon Mureinik commented on CSV-252: I've posted a PR to perform this upgrade: [https://github.com/apache/commons-csv/pull/49] > Upgrade test framework to JUnit Jupiter > --- > > Key: CSV-252 > URL: https://issues.apache.org/jira/browse/CSV-252 > Project: Commons CSV > Issue Type: Task > Components: Build > Reporter: Allon Mureinik >Priority: Major > Time Spent: 10m > Remaining Estimate: 0h > > Commons CSV's tests use JUnit 4.12, which is a tad outdated. > > We should upgrade to the modern JUnit Jupiter and make it easier to write > future tests. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Created] (CSV-252) Upgrade test framework to JUnit Jupiter
Allon Mureinik created CSV-252: -- Summary: Upgrade test framework to JUnit Jupiter Key: CSV-252 URL: https://issues.apache.org/jira/browse/CSV-252 Project: Commons CSV Issue Type: Task Components: Build Reporter: Allon Mureinik Commons CSV's tests use JUnit 4.12, which is a tad outdated. We should upgrade to the modern JUnit Jupiter and make it easier to write future tests. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (IO-628) Migration Commons-IO to JUnit Jupiter
[ https://issues.apache.org/jira/browse/IO-628?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16941315#comment-16941315 ] Allon Mureinik commented on IO-628: --- I posted [https://github.com/apache/commons-io/pull/97] in an attempt to tackle this task. Reviews are welcome. > Migration Commons-IO to JUnit Jupiter > - > > Key: IO-628 > URL: https://issues.apache.org/jira/browse/IO-628 > Project: Commons IO > Issue Type: Task > Components: Utilities > Reporter: Allon Mureinik >Priority: Minor > Time Spent: 10m > Remaining Estimate: 0h > > Migrate the Commons IO project to the modern JUnit Jupiter framework (instead > of JUnit 4 which it currently uses) in order to make it easier to maintain > and for future contributors to participate. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Created] (IO-628) Migration Commons-IO to JUnit Jupiter
Allon Mureinik created IO-628: - Summary: Migration Commons-IO to JUnit Jupiter Key: IO-628 URL: https://issues.apache.org/jira/browse/IO-628 Project: Commons IO Issue Type: Task Components: Utilities Reporter: Allon Mureinik Migrate the Commons IO project to the modern JUnit Jupiter framework (instead of JUnit 4 which it currently uses) in order to make it easier to maintain and for future contributors to participate. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] commons-lang issue #376: Test cleanup
Github user mureinik commented on the issue: https://github.com/apache/commons-lang/pull/376 > Looks great, thank you very much! ð > > There are just some checkstyle errors related to unused imports left to fix (see failed travis build). Missed a couple of unused import statements after I cleaned up the usages of `assertTrue` and `assertFalse`. I removed them, and the build should pass now. ---
[GitHub] commons-lang pull request #376: Test cleanup
GitHub user mureinik opened a pull request: https://github.com/apache/commons-lang/pull/376 Test cleanup Following up after the migration of the test suite to JUnit Jupiter, some test code could be cleaned up to make it clearer and easier to maintain. This PR contains several patches around that area: General fixes: - In order to test an expected exception, `assertThrows` was used instead of a cumbersome `if`-`fail`-`catch` construct. - In order to fail a test on an unexpected exception, the exception is thrown outside the method, instead of the cumbersome construct of catching it and calling `fail` - Redundant `throws` clauses were removed - `assertEquals`, `assertNotEquals`, `assertSame`, `assertNotSame`, `assertNull` and `assertNotNull` were used instead of reimplementing them with conditions in `if` statements or `assertTrue`s. Specific improvements: - `ConverstionTest#assertBinaryEquals` was removed in favor of the built-in `Assertions#assertArraysEquals(boolean[], boolean[])`. - `LocaleUtilsTest#parseAllLocales` was rewritten as a parameterized test instead of implementing it manually with a loop. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mureinik/commons-lang test-cleanup Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-lang/pull/376.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #376 commit c9ee985fce7d14d673d9b6bace824560eb4d40fc Author: Allon Mureinik Date: 2018-10-12T15:12:34Z Remove ConversionTest#assertBinaryEquals JUnit Jupiter's Assertions has an assertArraysEuqals(boolean[], boolean[]) method, so there's no longer a need for the assertBinaryEquals method. commit cf44c603b105f154b6b57604fe9abd589b7dbd2b Author: Allon Mureinik Date: 2018-10-12T16:14:01Z Make LocaleUtilsTest#parseAllLocales parameterized This patch converts testParseAllLocales to a @ParameterizedTest instead of iterating over the locales inside the method's body. This changes allows using standard asserts for each case individually instead of having to count failures and print the problematic locales to System.out. commit 15daf92088ad6d8868cd157ca9666721a59ce705 Author: Allon Mureinik Date: 2018-10-13T14:16:54Z Remove double stop() test in StopWatchTest StopWatchTest#testBadStates has the same block of code testing StopWatch#stop copy-pasted. This patch cleans it up by removing one of those blocks. commit 8507e5c81a8d3fedc655c02c93d4cf9dd4418ff6 Author: Allon Mureinik Date: 2018-10-13T14:08:48Z Clean up testing of exceptions Now that the entire project is ported to JUnit Jupiter, there are more elegant ways to test for exceptions, which this patch applies throughtout the code base. If throwing an exception is supposed to fail a test, just throwing it outside of the method cleans up the code and makes it more elegant, instead of catching it and calling Assertions#fail. If an exception is supposed to be thrown, calling Assertions#assertThrows is a more elegant option than calling Assertions#fail in the try block and then catching and ignoring the expected exception. Note that assertThrows uses a lambda block, so the variables inside it should be final or effectively final. Reusing variables is a common practice in the tests, so where needed new final variables were introduced, or the variables used were inlined. commit 3c6141d401233176d2e424640bffe0369592349e Author: Allon Mureinik Date: 2018-10-13T16:38:01Z Use assertTrue/assertFalse instead of reimplementing them Use assertTrue and assertFalse instead of reimplemeting their logic by having an if statement conditionalling call fail. commit 8ee1a558b821f28313b8b538b5d4b0de1b0e7044 Author: Allon Mureinik Date: 2018-10-13T16:49:52Z Clean up redundant throws clauses commit 591bebc111bca4f0681560b70fcc3d20cda40577 Author: Allon Mureinik Date: 2018-10-13T17:05:40Z Clean up assertions Use built-in assertion methods provides by org.junit.jupiter.api.Assertions instead of reimplementing the same logic with assertTrue and assertFalse. Note that JUnit Jupiter 5.3.1 does not support deltas of 0 for assertEquals(double, double, double) and assertEquals(float, float, float), so these usages of assertTrue were left untouched, and will be addressed when JUnit Jupiter 5.4, that addresses this isssue, will be available (see https://github.com/junit-team/junit5/pull/1613 for details). ---
[GitHub] commons-lang pull request #375: JUnit Jupiter migration completion
GitHub user mureinik opened a pull request: https://github.com/apache/commons-lang/pull/375 JUnit Jupiter migration completion This patch finalizes the upgrade of commons-lang's tests to use JUnit Jupiter and removes the Vintage Engine dependency entirely. While most of these changes are drop-in replacements with no functional benefit, there are some non-obvious changes worth mentioning. Unlike `org.junit.Assert.assertEquals(double, double, double)`, `org.junit.jupiter.api.Assertions.assertEquals(double, double, double)` does not support deltas of zero, only strictly positive deltas. This issue will be addressed in JUnit Jupiter 5.4 (see https://github.com/junit-team/junit5/pull/1613 for details). In the meanwhile, `assertTrue(expected==actual)` was used, and `TODO` comments were placed in the code to refactor it to assertEquals once JUnit 5.4 is available. Unlike `org.junit.Test`, `org.junit.jupiter.api.Test` does not have an `expected` argument. Instead, an explicit call to `org.junit.jupiter.api.Assertions.assertThrows` is used. Unlike `org.junit.Test`, `org.junit.jupiter.api.Test` does not have a `timeout` argument either. Instead, an explicit call to `org.junit.jupiter.api.Assertions.assertTimeoutPreemptively` is used. JUnit Jupiter also no longer has the concept of Rules. Usages of the `SystemDefaultsSwitch` rule and its accompanying annotates were replaced with the `@DefaultLocale` annotation that @britter contributed to JUnit Pioneer, the semi-official JUnit extension project, or simply removed entirely where they had no use (i.e., from `StringUtilsEqulasIndexOfTest`). Following the removal of their usages, the `@SystemDefaults` annotation, the `SystemDefaultsSwitch` rule and the `SystemDefaultsSwitchTest` class that tests them had no more use, and they were removed entirely. It's also worth noting this is a minimal patch for migrating the package's tests to Jupiter. There are several tests that can be made more elegant with Jupiter's new features, but that work is left for subsequent patches. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mureinik/commons-lang junit-jupiter Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-lang/pull/375.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #375 commit 57fbe7e96eec45507dcbe030c3d9ea975f33342f Author: Allon Mureinik Date: 2018-10-11T13:21:52Z StringUtilsEqulasIndexOfTest @Rules Commit 0223a4 removed all the usages of @SystemDefaults annotations from this test, but left behind a pointless SystemDefaultsSwitch rule. This patch finishes the job and removes this unused rule. commit 9a3e1d743bc4c8563cf71c521b4d392e302dfbc6 Author: Allon Mureinik Date: 2018-10-02T03:41:37Z Update tests to JUnit Jupiter This patch finalizes the upgrade of commons-lang's tests to use JUnit Jupiter and remove the Vintage Engine dependency entirely. While most of these changes are drop-in replacements with no functional benefit, there are some non-obvious changes worth mentioning. Unlike org.junit.Assert.assertEquals(double, double, double), org.junit.jupiter.api.Assertions.assertEquals(double, double, double) does not support deltas of zero, only strictly positive deltas. This issue will be addressed in JUnit Jupiter 5.4 (see https://github.com/junit-team/junit5/pull/1613 for details). In the meanwhile, assertTrue(expected==actual) was used, and TODO comments were placed in the code to refactor it to assertEquals once JUnit 5.4 is available. Unlike org.junit.Test, org.junit.jupiter.api.Test does not have an "expected" argument. Instead, an explicit call to org.junit.jupiter.api.Assertions.assertThrows is used. Unlike org.junit.Test, org.junit.jupiter.api.Test does not have a "timeout" argument either. Instead, an explicit call to org.junit.jupiter.api.Assertions.assertTimeoutPreemptively is used. JUnit Jupiter also no longer has the concept of Rules. Usages of the SystemDefaultsSwitch rule and its accompanying annotates were replaced with the @DefaultLocale annotation that Benedikt Ritter contributed to JUnit Pioneer, the semi-official JUnit extension project. Following the removal of their usages, the SystemDefaults annotation, the SystemDefaultsSwitch rule and the SystemDefaultsSwitchTest class that tests them had no more use, and they were removed entirely. It's also worth noting this is a minimal patch for migrating the package's tests to Jupiter. There are several tests that can be made more elegant with Jupiter's new features, but that work is left for subsequent patches. ---
[jira] [Updated] (SCXML-284) Clear up exception handling in tests
[ https://issues.apache.org/jira/browse/SCXML-284?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Allon Mureinik updated SCXML-284: - Flags: Patch > Clear up exception handling in tests > > > Key: SCXML-284 > URL: https://issues.apache.org/jira/browse/SCXML-284 > Project: Commons SCXML > Issue Type: Task >Reporter: Allon Mureinik >Priority: Minor > > Following SCXML-283, JUnit Jupiter provides some tools to handle exception > testing in a more elegant way than catching an exception and asserting its > properties in that block -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] commons-scxml pull request #4: SCXML-284 Clean up exception handling in test...
GitHub user mureinik opened a pull request: https://github.com/apache/commons-scxml/pull/4 SCXML-284 Clean up exception handling in tests This PR cleans up the handling of exceptions in the test code: 1. The cumbersome idiom of `try`-`catch` with a `fail` call in the `try` block and additional assertions in the `catch` block were replaced with Jupiter's elegant `Assertions#assertThrows` call. 2. Redudant `throws` clauses were removed 3. `Issue112Test#test01issue112`'s code was cleaned up so it actually fails if an exception is thrown instead of catching it and printing it to `System.err`. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mureinik/commons-scxml test-exceptions Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-scxml/pull/4.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #4 commit a59d8f1b4504e30b36c7a22ef2e087cbd98a7553 Author: Allon Mureinik Date: 2018-10-10T16:38:30Z SCXML-284 Issue112Test#test01issue112 catch In case an event can't be fired, something is wrong, and the test should fail, not catch and ignore the exception. commit 6c5932ac952a1b3d335fa7217f3b54799321dd95 Author: Allon Mureinik Date: 2018-10-10T16:45:44Z SCXML-284 Replace try-catch with assertThrows Use JUnit Jupiter's new mechanism of assertThrows instead of try-catch blocks in order to clean up the code and make it easier to follow. Since the questionable piece of code is now run inside a lambda and is caught by asserThrows itself, there's no need to declare these exceptions in the throws clause of the mehod declarations, and they were cleaned up to avoid adding compiler warnings. commit b2b670f8b8e0ff1a673d692be2ea38cb5fc54e66 Author: Allon Mureinik Date: 2018-10-10T16:50:17Z SCXML-284 Remove redundant throws declarations in tests ---
[jira] [Created] (SCXML-284) Clear up exception handling in tests
Allon Mureinik created SCXML-284: Summary: Clear up exception handling in tests Key: SCXML-284 URL: https://issues.apache.org/jira/browse/SCXML-284 Project: Commons SCXML Issue Type: Task Reporter: Allon Mureinik Following SCXML-283, JUnit Jupiter provides some tools to handle exception testing in a more elegant way than catching an exception and asserting its properties in that block -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] commons-lang issue #374: Update time tests to JUnit Jupiter
Github user mureinik commented on the issue: https://github.com/apache/commons-lang/pull/374 Updated the patch to fix the scoping issue in `pom.xml` @PascalSchumacher commented about. ---
[GitHub] commons-lang pull request #374: Update time tests to JUnit Jupiter
Github user mureinik commented on a diff in the pull request: https://github.com/apache/commons-lang/pull/374#discussion_r224144394 --- Diff: pom.xml --- @@ -543,6 +543,11 @@ junit-vintage-engine test + + org.junit-pioneer + junit-pioneer + 0.2.0 --- End diff -- Good catch, thanks. ---
[GitHub] commons-lang issue #374: Update time tests to JUnit Jupiter
Github user mureinik commented on the issue: https://github.com/apache/commons-lang/pull/374 @britter I was half way through rewriting the `SystemDefaultsSwitch` as a JUnit Jupiter extension when I remembered your tweet from earlier this month and figured out that you already did this work in JUnit Pioneer. I hope I'm not stepping on any toes here, but if this conflicts with anything you had planned, I'd gladly abandon this PR. ---
[GitHub] commons-lang pull request #374: Update time tests to JUnit Jupiter
GitHub user mureinik opened a pull request: https://github.com/apache/commons-lang/pull/374 Update time tests to JUnit Jupiter Upgrade the tests in the `time` package to use JUnit Jupiter as part of the effort to remove the dependency on the Vintage Engine. While most of these changes are drop-in replacements with no functional benefit, there are some non-obvious changes worth mentioning. Unlike `org.junit.Test`, `org.junit.jupiter.api.Test` does not have an `expected` argument. Instead, an explicit call to `org.junit.jupiter.api.Assertions.assertThrows` is used. JUnit Jupiter no longer has a concept of runners. Tests previously run with the `org.junit.runners.Parameterized` runner were rewritten to use `@ParameterizedTest` and a `@MethodSource`. JUnit Jupiter also no longer has the concept of Rules. Usages of the `SystemDefaultsSwitch` rule and its accompanying annotates were replaced with the `@DefaultLocale` and `@DefaultTimezone` annotations that @britter contributed to [JUnit Pioneer](https://github.com/junit-pioneer/junit-pioneer), the semi-official JUnit extension project. It's also worth noting this is a minimal patch for migrating the package's tests to Jupiter. There are several tests that can be made more elegant with Jupiter's new features, but that work is left for subsequent patches. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mureinik/commons-lang junit-jupiter-time Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-lang/pull/374.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #374 commit 74a18ce4c6ff7e7f5db9ed788d5f4999f7213cfe Author: Allon Mureinik Date: 2018-10-02T03:41:37Z Update time tests to JUnit Jupiter Upgrade the tests in the time package to use JUnit Jupiter as part of the effort to remove the dependency on the Vintage Engine. While most of these changes are drop-in replacements with no functional benefit, there are some non-obvious changes worth mentioning. Unlike org.junit.Test, org.junit.jupiter.api.Test does not have an "expected" argument. Instead, an explicit call to org.junit.jupiter.api.Assertions.assertThrows is used. JUnit Jupiter no longer has a concept of runners. Tests previously run with the org.junit.runners.Parameterized runner were rewritten to use @ParameterizedTest and a @MethodSource. JUnit Jupiter also no longer has the concept of Rules. Usages of the SystemDefaultsSwitch rule and its accompanying annotates were replaced with the @DefaultLocale and @DefaultTimezone annotations that Benedikt Ritter contributed to JUnit Pioneer, the semi-official JUnit extension project. It's also worth noting this is a minimal patch for migrating the package's tests to Jupiter. There are several tests that can be made more elegant with Jupiter's new features, but that work is left for subsequent patches. ---
[GitHub] commons-lang pull request #373: Update mutable tests to JUnit Jupiter
GitHub user mureinik opened a pull request: https://github.com/apache/commons-lang/pull/373 Update mutable tests to JUnit Jupiter Upgrade the tests in the `mutable` package to use JUnit Jupiter as part of the effort to remove the dependency on the Vintage Engine. While most of these changes are drop-in replacements with no functional benefit, there are some non-obvious changes worth mentioning. Unlike `org.junit.Test`, `org.junit.jupiter.api.Test` does not have an `expected` argument. Instead, an explicit call to `org.junit.jupiter.api.Assertions.assertThrows` is used. Unlike `org.junit.Assert.assertEquals(double, double, double)`, `org.junit.jupiter.api.Assertions.assertEquals(double, double, double)` does not support deltas of zero, only strictly positive deltas. This issue will be addressed in JUnit Jupiter 5.4 (see https://github.com/junit-team/junit5/pull/1613 for details). In the meanwhile, `assertTrue(expected==actual)` was used, and `TODO` comments were placed in the code to refactor it to assertEquals once JUnit 5.4 is available. It's also worth noting this is a minimal patch for migrating the package's tests to Jupiter. There are several tests that can be made more elegant with Jupiter's new features, but that work is left for subsequent patches. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mureinik/commons-lang junit-jupiter-mutable Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-lang/pull/373.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #373 commit 94beded83960af2a15992f6cec8f1f59d1f3c051 Author: Allon Mureinik Date: 2018-10-02T03:41:37Z Update mutable tests to JUnit Jupiter Upgrade the tests in the mutable package to use JUnit Jupiter as part of the effort to remove the dependency on the Vintage Engine. While most of these changes are drop-in replacements with no functional benefit, there are some non-obvious changes worth mentioning. Unlike org.junit.Test, org.junit.jupiter.api.Test does not have an "expected" argument. Instead, an explicit call to org.junit.jupiter.api.Assertions.assertThrows is used. Unlike org.junit.Assert.assertEquals(double, double, double), org.junit.jupiter.api.Assertions.assertEquals(double, double, double) does not support deltas of zero, only strictly positive deltas. This issue will be addressed in JUnit Jupiter 5.4 (see https://github.com/junit-team/junit5/pull/1613 for details). In the meanwhile, assertTrue(expected==actual) was used, and TODO comments were placed in the code to refactor it to assertEquals once JUnit 5.4 is available. It's also worth noting this is a minimal patch for migrating the package's tests to Jupiter. There are several tests that can be made more elegant with Jupiter's new features, but that work is left for subsequent patches. ---
[GitHub] commons-lang pull request #372: Update math tests to JUnit Jupiter
GitHub user mureinik opened a pull request: https://github.com/apache/commons-lang/pull/372 Update math tests to JUnit Jupiter Upgrade the tests in the `math` package to use JUnit Jupiter as part of the effort to remove the dependency on the Vintage Engine. While most of these changes are drop-in replacements with no functional benefit, there are some non-obvious changes worth mentioning. Unlike `org.junit.Test`, `org.junit.jupiter.api.Test` does not have an `expected` argument. Instead, an explicit call to `org.junit.jupiter.api.Assertions.assertThrows` is used. Unlike `org.junit.Assert.assertEquals(double, double, double)`, `org.junit.jupiter.api.Assertions.assertEquals(double, double, double)` does not support deltas of zero, only strictly positive deltas. This issue will be addressed in JUnit Jupiter 5.4 (see https://github.com/junit-team/junit5/pull/1613 for details). In the meanwhile, `assertTrue(expected==actual)` was used, and `TODO` comments were placed in the code to refactor it to assertEquals once JUnit 5.4 is available. It's also worth noting this is a minimal patch for migrating the package's tests to Jupiter. There are several tests that can be made more elegant with Jupiter's new features, but that work is left for subsequent patches. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mureinik/commons-lang junit-jupiter-math Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-lang/pull/372.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #372 commit 47a9ea7c828772625b9e26c0c7a6db494ea626a3 Author: Allon Mureinik Date: 2018-10-02T03:41:37Z Update math tests to JUnit Jupiter Upgrade the tests in the math package to use JUnit Jupiter as part of the effort to remove the dependency on the Vintage Engine. While most of these changes are drop-in replacements with no functional benefit, there are some non-obvious changes worth mentioning. Unlike org.junit.Test, org.junit.jupiter.api.Test does not have an "expected" argument. Instead, an explicit call to org.junit.jupiter.api.Assertions.assertThrows is used. Unlike org.junit.Assert.assertEquals(double, double, double), org.junit.jupiter.api.Assertions.assertEquals(double, double, double) does not support deltas of zero, only strictly positive deltas. This issue will be addressed in JUnit Jupiter 5.4 (see https://github.com/junit-team/junit5/pull/1613 for details). In the meanwhile, assertTrue(expected==actual) was used, and TODO comments were placed in the code to refactor it to assertEquals once JUnit 5.4 is available. It's also worth noting this is a minimal patch for migrating the package's tests to Jupiter. There are several tests that can be made more elegant with Jupiter's new features, but that work is left for subsequent patches. ---
[GitHub] commons-lang pull request #371: Update tuple tests to JUnit Jupiter
GitHub user mureinik opened a pull request: https://github.com/apache/commons-lang/pull/371 Update tuple tests to JUnit Jupiter Upgrade the tests in the text package to use JUnit Jupiter as part of the effort to remove the dependency on the Vintage Engine. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mureinik/commons-lang junit-jupiter-tuple Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-lang/pull/371.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #371 commit f110da945a8af0914f348cfe576b5a8257159fd9 Author: Allon Mureinik Date: 2018-10-02T03:41:37Z Update tuple tests to JUnit Jupiter Upgrade the tests in the text package to use JUnit Jupiter as part of the effort to remove the dependency on the Vintage Engine. ---
[GitHub] commons-lang pull request #370: Update text tests to JUnit Jupiter
GitHub user mureinik opened a pull request: https://github.com/apache/commons-lang/pull/370 Update text tests to JUnit Jupiter Upgrade the tests in the `text` package to use JUnit Jupiter as part of the effort to remove the dependency on the Vintage Engine. While most of these changes are drop-in replacements with no functional benefit, there are some non-obvious changes worth mentioning. Unlike `org.junit.Test`, `org.junit.jupiter.api.Test` does not have an `expected` argument. Instead, an explicit call to `org.junit.jupiter.api.Assertions.assertThrows` is used. It's also worth noting this is a minimal patch for migrating the package's tests to Jupiter. There are several tests that can be made more elegant with Jupiter's new features, but that work is left for subsequent patches. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mureinik/commons-lang junit-jupiter-text Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-lang/pull/370.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #370 commit 182e335432fe4634d770c5861be40dacf1816f0c Author: Allon Mureinik Date: 2018-10-02T03:41:37Z Update text tests to JUnit Jupiter Upgrade the tests in the text package to use JUnit Jupiter as part of the effort to remove the dependency on the Vintage Engine. While most of these changes are drop-in replacements with no functional benefit, there are some non-obvious changes worth mentioning. Unlike org.junit.Test, org.junit.jupiter.api.Test does not have an "expected" argument. Instead, an explicit call to org.junit.jupiter.api.Assertions.assertThrows is used. It's also worth noting this is a minimal patch for migrating the package's tests to Jupiter. There are several tests that can be made more elegant with Jupiter's new features, but that work is left for subsequent patches. ---
[GitHub] commons-lang pull request #369: Update reflect tests to JUnit Jupiter
GitHub user mureinik opened a pull request: https://github.com/apache/commons-lang/pull/369 Update reflect tests to JUnit Jupiter Upgrade the tests in the `reflect` package to use JUnit Jupiter as part of the effort to remove the dependency on the Vintage Engine. While most of these changes are drop-in replacements with no functional benefit, there are some non-obvious changes worth mentioning. Unlike `org.junit.Test`, `org.junit.jupiter.api.Test` does not have an `expected` argument. Instead, an explicit call to `org.junit.jupiter.api.Assertions.assertThrows` is used. Unlike `org.junit.Assume`, `org.junit.jupiter.api.Assumptions` does not have an `assumeNotNull` method. Instead, `assumeTrue` was used with an explicit condition of a variable being different than `null`. It's also worth noting this is a minimal patch for migrating the package's tests to Jupiter. There are several tests that can be made more elegant with Jupiter's new features, but that work is left for subsequent patches. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mureinik/commons-lang junit-jupiter-reflect Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-lang/pull/369.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #369 commit cbc8e0b2952164f97779c9a5fadca6acd2600ae2 Author: Allon Mureinik Date: 2018-10-02T03:41:37Z Update reflect tests to JUnit Jupiter Upgrade the tests in the reflect package to use JUnit Jupiter as part of the effort to remove the dependency on the Vintage Engine. While most of these changes are drop-in replacements with no functional benefit, there are some non-obvious changes worth mentioning. Unlike org.junit.Test, org.junit.jupiter.api.Test does not have an "expected" argument. Instead, an explicit call to org.junit.jupiter.api.Assertions.assertThrows is used. Unlike org.junit.Assume, org.junit.jupiter.api.Assumptions does not have an assumtNotNull method. Instead, assumeTrue was used with an explicit condition of a variable being different than null. It's also worth noting this is a minimal patch for migrating the package's tests to Jupiter. There are several tests that can be made more elegant with Jupiter's new features, but that work is left for subsequent patches. ---
[GitHub] commons-lang pull request #368: Update exception tests to JUnit Jupiter
GitHub user mureinik opened a pull request: https://github.com/apache/commons-lang/pull/368 Update exception tests to JUnit Jupiter Upgrade the tests in the `exception` package to use JUnit Jupiter as part of the effort to remove the dependency on the Vintage Engine. While most of these changes are drop-in replacements with no functional benefit, there are some non-obvious changes worth mentioning. Unlike `org.junit.Test`, `org.junit.jupiter.api.Test` does not have an `expected` argument. Instead, an explicit call to `org.junit.jupiter.api.Assertions.assertThrows` is used. Another non-obvious change was performed in `ContextedRuntimeExceptionTest`. Unlike JUnit Vintage's `@Before`, JUnit Jupiter's `@BeforeEach` does not apply if a parent's method is annotated with it and the overriding method is not, so an explicit `@BeforeEach` annotation had to be added to `ContexedTuntimeExceptionTest#setUp()`. It's also worth noting this is a minimal patch for migrating the package's tests to Jupiter. There are several tests that can be made more elegant with Jupiter's new features, but that work is left for subsequent patches. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mureinik/commons-lang junit-jupiter-exception Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-lang/pull/368.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #368 commit 884d273f4207095f881167b3398fc2a55617ee9a Author: Allon Mureinik Date: 2018-10-02T03:41:37Z Update exception tests to JUnit Jupiter Upgrade the tests in the exception package to use JUnit Jupiter as part of the effort to remove the dependency on the Vintage Engine. While most of these changes are drop-in replacements with no functional benefit, there are some non-obvious changes worth mentioning. Unlike org.junit.Test, org.junit.jupiter.api.Test does not have an "expected" argument. Instead, an explicit call to org.junit.jupiter.api.Assertions.assertThrows is used. Another non-obvious change was performed in ContextedRuntimeExceptionTest. Unlike JUnit Vintages's @Before, JUnit Jupiter's @BeforeEach does not apply if a parent's method is annotated with it and the overriding method is not, so an explicit @BeforeEach annotation had to be added to ContexedTuntimeExceptionTest#setUp(). It's also worth noting this is a minimal patch for migrating the package's tests to Jupiter. There are several tests that can be made made more elegant with Jupiter's new features, but that work is left for subsequent patches. ---
[GitHub] commons-lang pull request #367: Update event tests to JUnit Jupiter
GitHub user mureinik opened a pull request: https://github.com/apache/commons-lang/pull/367 Update event tests to JUnit Jupiter Upgrade the tests in the event package to use JUnit Jupiter as part of the effort to remove the dependency on the Vintage Engine. While most of these changes are drop-in replacements with no functional benefit, it is worth noting the tests that test thrown exceptions. Prior to this patch, this package tested for exceptions in two ways, neither of which are supported in JUnit Jupiter: 1. With the `expected` argument of the `org.junit.Test` annotation. 2. With the `org.junit.rules.ExpectedException` `@Rule` Both of these usages were replaced with calls to `org.junit.jupiter.api.Assertions#assertThrows`. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mureinik/commons-lang junit-jupiter-event Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-lang/pull/367.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #367 commit 762641dcdbae9456aa2b72ec8fa1baa0acab942f Author: Allon Mureinik Date: 2018-10-02T03:41:37Z Update event tests to JUnit Jupiter Upgrade the tests in the event package to use JUnit Jupiter as part of the effort to remove the dependency on the Vintage Engine. While most of these changes are drop-in replacements with no functional benefit, it is worth noting the tests that test thrown excetpions. Prior to this patch, this package tested for exceptions in two ways, neither of which are supported in JUnit Jupiter: 1. With the "expected" argument of the org.junit.Test annotation. 2. With the org.junit.rules.ExpectedException @Rule Both of these usages were replaced with calls to org.junit.jupiter.api.Assertions#assertThrows. ---
[GitHub] commons-lang pull request #366: Update concurrent tests to JUnit Jupiter
GitHub user mureinik opened a pull request: https://github.com/apache/commons-lang/pull/366 Update concurrent tests to JUnit Jupiter Upgrade the tests in the concurrent package to use JUnit Jupiter as part of the effort to remove the dependency on the Vintage Engine. While most of these changes are drop-in replacements with no functional benefit, there are some non-obvious changes worth mentioning. Unlike `org.junit.Test`, `org.junit.jupiter.api.Test` does not have an `expected` argument. Instead, an explicit call to `org.junit.jupiter.api.Assertions.assertThrows` is used. This call allows the test to pinpoint the exact statement that is expected to throw the exception and allows making the tests a bit stricter by preventing false-positives that could occur if the setup code would throw the expected exception instead of the statement that was supposed to throw it. Another notable change was performed in `MemoizerTest`. JUnit Jupiter does not support JUnit 4's runners, and EasyMock has no equivalent `@Extension`, so a setup method was added and the mock was explicitly initialized. It's also worth noting this is a minimal patch for migrating the package's tests to Jupiter. There are several tests that can be made more elegant with Jupiter's new features, but that work is left for subsequent patches. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mureinik/commons-lang junit-jupiter-concurrent Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-lang/pull/366.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #366 commit dd761382d3cfdc11b5de0cb1246de4567cdf2fc2 Author: Allon Mureinik Date: 2018-10-02T03:41:37Z Update concurrent tests to JUnit Jupiter Upgrade the tests in the concurrent package to use JUnit Jupiter as part of the effort to remove the dependency on the Vintage Engine. While most of these changes are drop-in replacements with no functional benefit, there are some non-obvious changes worth mentioning. Unlike org.junit.Test, org.junit.jupiter.api.Test does not have an "expected" argument. Instead, an explicit call to org.junit.jupiter.api.Assertions.assertThrows is used. This call allows the test to pinpoint the exact statement that is expected to throw the exception and allows making the tests a bit stricter by preventing false-positives that could occur if the setup code would throw the expected exception instead of the statement that was supposed to throw it. Another notable change was performed in MemoizerTest. JUnit Jupiter does not support JUnit 4's runners, and EasyMock has no equivalent @Extension, so a setup method was added and the mock was explicitly initialized. It's also worth noting this is a minimal patch for migrating the package's tests to Jupiter. There are several tests that can be made made more elegant with Jupiter's new features, but that work is left for subsequent patches. ---
[GitHub] commons-lang pull request #363: Update builder tests to JUnit Jupiter
Github user mureinik closed the pull request at: https://github.com/apache/commons-lang/pull/363 ---
[GitHub] commons-lang issue #363: Update builder tests to JUnit Jupiter
Github user mureinik commented on the issue: https://github.com/apache/commons-lang/pull/363 > Thanks! ð > > Merged in [729adb6](https://github.com/apache/commons-lang/commit/729adb624d3e720afb8686093814ab2bcc2d2f13) > > Not sure why this pull request was not auto-closed even-though I fast-forward merged the branch. > > Could you please close the pull request? > > Thanks! Closing, thanks @PascalSchumacher ---
[jira] [Commented] (SCXML-283) Upgrade testing framework to JUnit Jupiter
[ https://issues.apache.org/jira/browse/SCXML-283?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16638676#comment-16638676 ] Allon Mureinik commented on SCXML-283: -- I've proposed [https://github.com/apache/commons-scxml/pull/3] to handle this upgrade. > Upgrade testing framework to JUnit Jupiter > -- > > Key: SCXML-283 > URL: https://issues.apache.org/jira/browse/SCXML-283 > Project: Commons SCXML > Issue Type: Task >Reporter: Allon Mureinik >Priority: Minor > > commons-scxml already requires Java 8, so there's nothing inhibiting the > upgrade to a modern JUnit version, and taking advantage of Jupiter's newer > features. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] commons-scxml issue #3: SCXML-283 Upgrade to JUnit jupiter
Github user mureinik commented on the issue: https://github.com/apache/commons-scxml/pull/3 @woonsan I've opened https://issues.apache.org/jira/browse/SCXML-283 for this task and updated the PR's message as well as the individual commit messages accordingly. ---
[jira] [Created] (SCXML-283) Upgrade testing framework to JUnit Jupiter
Allon Mureinik created SCXML-283: Summary: Upgrade testing framework to JUnit Jupiter Key: SCXML-283 URL: https://issues.apache.org/jira/browse/SCXML-283 Project: Commons SCXML Issue Type: Task Reporter: Allon Mureinik commons-scxml already requires Java 8, so there's nothing inhibiting the upgrade to a modern JUnit version, and taking advantage of Jupiter's newer features. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] commons-scxml issue #3: Upgrade to JUnit jupiter
Github user mureinik commented on the issue: https://github.com/apache/commons-scxml/pull/3 @woonsan Will do, and I'll update the PR with its details once its done ---
[GitHub] commons-scxml pull request #3: Upgrade to JUnit jupiter
GitHub user mureinik opened a pull request: https://github.com/apache/commons-scxml/pull/3 Upgrade to JUnit jupiter This PR migrates the project to the modern JUnit Jupiter 5.3.1 testing framework. Since JUnit Jupiter is not backwards compatible to JUnit 4.x, or even JUnit Vintage 5.x, the patch is a tad large, although most of the changes are just cosmetic. 1. Maven changes: 1. `org.junit.jupiter:junit-jupiter-api` was added to provide the new APIs used. 1. `org.junit.jupiter:junit-jupiter-engine` was added as a testing engine. Its presence allows `maven-surefire-plugin` to use the Jupiter provider in order to execute tests. 1. `junit:junit` was removed, as it's no longer in use. 1. The parent module, `org.apache.commons:commons-parent` was upgraded to the latest version, `47`, in order to consume `org.apache.maven.plugins:maven-surefire-plugin:2.22.0` that natively supports Jupiter tests. 1. Annotations: 1. `org.junit.jupiter.api.Test` was used as a drop in replacement for `org.juit.Test` without arguments. See 3.b for handling of the `@Test` annotation with an `expected` argument. 1. `org.junit.jupiter.api.BeforeEach` was used as an drop in replacement for `org.junit.Before`. 1. `org.junit.jupiter.api.AfterEach` was used as a drop in replacement for `org.junit.After`. 1. `org.junit.jupiter.api.BeforeAll` was used as a drop in replacement for `org.junit.BeforeClass`. 1. `org.junit.jupiter.api.AfterAll` was used as a drop in replacement for `org.junit.AfterClass`. 3. Assertions: 1. `org.junit.jupiter.api.Assertions`' methods were used as a drop in replacements for `org.junit.Assert`'s methods with the same name in the simple case of an assertion without a message. In the case of an assertion with a message, `org.junit.jupiter.api.Assertions`' methods were used, but the argument order was changed - `Assert`'s methods take the message as the first argument, while `Assertions`' methods take the message as the last argument. 1. `org.junit.jupiter.api.Assertions#assertThrows` was used to assert a specific exception was thrown instead of an `org.junit.Test` annotation with an `expected` argument. This technique has a side bonus of making the tests slightly stricter, as now they assert the exception was thrown from a specific line and prevent false posivites where the test's "set-up" code accidentally threw that exception. The `throws` clauses of these methods were cleaned up from exceptions that can no longer be thrown in order to avoid compilation warnings. 4. Miscelanious 1. The redundant `main` methods in the test classes of the `org.apache.commons.scxml2.env.javascript` package were removed, and the Javadoc was updated accordingly. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mureinik/commons-scxml junit-jupiter Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-scxml/pull/3.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #3 commit 3d133ce5490a26252622c14d34460501770506e0 Author: Allon Mureinik Date: 2018-10-04T09:55:34Z Remove unused main methods Remove unused main methods of in the org.apache.commons.scxml2.env.javascript package. These tests do not need tham, as they are run via maven-surefire-plugin which picks up the annotated tests. commit 73462b0538755f838ea47fd6c83e31b393e6762d Author: Allon Mureinik Date: 2018-10-04T09:56:36Z javadoc fix Fix org.apache.commons.scxml2.env.javascript tests' javadocs - the test methods are clearly annotaed with JUnit 4 annotations, and therefore these are not a JUnit 3 tests. commit 521518be32d3b1ab1f28e3509347702c1d7a3c8d Author: Allon Mureinik Date: 2018-10-04T11:04:03Z org.apache.commons:commons-parent dependency Upgraded to the latest available org.apache.commons:commons-parent in order to consume the latest maven-surefire-plugin required for the JUnit Jupiter upgarde. commit 8444b30a11e87e8b8e990d9c145ea4b1e617a0d0 Author: Allon Mureinik Date: 2018-10-04T10:59:39Z Upgrade to JUnit Jupiter This PR migrates the project to the modern JUnit Jupiter 5.3.1 testing framework. Since JUnit Jupiter is not backwards compatible to JUnit 4.x, or even JUnit Vintage 5.x, the patch is a tad large, although most of the changes are just cosmetic. 1. Maven changes: a. org.junit.jupiter:junit-jupiter-api was added to provide the new APIs used. b. org.junit.jupiter:junit-jupiter-engine was added as a testing engine. Its presence allows maven-surefire-plugin to use the Jupiter provider in order to execute tes
[GitHub] commons-lang pull request #363: Update builder tests to JUnit Jupiter
GitHub user mureinik opened a pull request: https://github.com/apache/commons-lang/pull/363 Update builder tests to JUnit Jupiter Upgrade the tests in the builder package to use JUnit Jupiter as part of the effort to remove the dependency on the Vintage Engine. While most of these changes are drop-in replacements with no functional benefit, it is worth mentioning the change to how expected exceptions are tested. Unlike `org.junit.Test`, `org.junit.jupiter.api.Test` does not have an `expected` argument. Instead, an explicit call to `org.junit.jupiter.api.Assertions.assertThrows` is used. This call allows the test to pinpoint the exact statement that is expected to throw the exception and allows making the tests a bit stricter by preventing false-positives that could occur if the setup code would throw the expected exception instead of the statement that was supposed to throw it. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mureinik/commons-lang junit-jupiter Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-lang/pull/363.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #363 commit 0c320dfc356b78145107a4b121d674dbccb84437 Author: Allon Mureinik Date: 2018-10-02T03:41:37Z Update builder tests to JUnit Jupiter Upgrade the tests in the builder package to use JUnit Jupiter as part of the effort to remove the dependency on the Vintage Engine. While most of these changes are drop-in replacements with no functional benefit, it is worth mentioning the change to how expected exceptions are tested. Unlike org.junit.Test, org.junit.jupiter.api.Test does not have an "expected" argument. Instead, an explicit call to org.junit.jupiter.api.Assertions.assertThrows is used. This call allows the test to pinpoint the exact statement that is expected to throw the expcetion and allows making the tests a bit stricter by preventing false-positives that could occur if the setup code would throw the expected exception instead of the statement that was supposed to throw it. ---
[GitHub] commons-lang pull request #321: SerializationUtilsTest cleanup
GitHub user mureinik opened a pull request: https://github.com/apache/commons-lang/pull/321 SerializationUtilsTest cleanup This PR includes several improvements to `SerializationUtilsTest` that both clean up the code and make the error message in case of a failure clearer by using JUnit's built-in capabilities. Changes include: - replacing `assertTrue(x != y)` with `assertNotSame(x, y)`. - Using the `expected` argument of the `@Test` annotation to test expected exceptions - Using `assertArraysEquals` to compare arrays You can merge this pull request into a Git repository by running: $ git pull https://github.com/mureinik/commons-lang SerializationUtilsTest Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-lang/pull/321.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #321 commit 4442cb2caeb82828ef91beea63d3b43bdb38c55e Author: Allon Mureinik <amureini@...> Date: 2018-04-04T03:58:09Z SerializationUtilsTest identity assertions Replaced calls to assertTrue with a != condition with calls to assertNotSame calls. This change retains the functionality, but will produce a more detailed error message in case the assertion fails. It also (arguably) makes the test code more straight-forward. commit fb963755e5ae54adaf6d632c1d013730356fdd46 Author: Allon Mureinik <amureini@...> Date: 2018-04-04T04:07:20Z SerializationUtilsTest expected exceptions Use the expected argument of the @Test annotation instead of boiler-plate implementing this behavior with a try-catch-fail construct in order to clean up the code and make it more obvious to the reader. commit 264b3f45f4031f0b3c235761aa87e5b2c5ace0e5 Author: Allon Mureinik <amureini@...> Date: 2018-04-04T04:10:28Z SerializatoinUtilsTest assertArraysEquals Utilize assertArraysEquals to compare arrays instead of boiler plate implementing it with a for loop. This change both makes the test code cleaner and improves the output in case of an assertion failure by showing all the differences between the two arrays instead of stopping at the first. ---
[GitHub] commons-lang pull request #316: Remove inequality check from shuffle tests
Github user mureinik closed the pull request at: https://github.com/apache/commons-lang/pull/316 ---
[GitHub] commons-lang issue #317: Predictable randomness in shuffle tests
Github user mureinik commented on the issue: https://github.com/apache/commons-lang/pull/317 This is an alternative approach to #316, and is discussed in the thread started in http://mail-archives.apache.org/mod_mbox/commons-dev/201802.mbox/%3CCAO2EVT7zk8j3-wPU54jGNKeLs%3Dyok-puAgNXg%3DGLEj38abUzHQ%40mail.gmail.com%3E ---
[GitHub] commons-lang pull request #316: Remove inequality check from shuffle tests
GitHub user mureinik opened a pull request: https://github.com/apache/commons-lang/pull/316 Remove inequality check from shuffle tests The `ArrayUtils.shuffle` methods could conceivably shuffle an array in a way that leaves the shuffled array equal to the original array, and thus asserting that the shuffled array differs from the original array in the test only passes statistically. The chance of this happening increases as an inverse of the number of distinct values in the array. E.g., in the edge case of an array where all the elements are equal to each other, shuffling the array has a 100% chance of producing an array that's equal to the original array. A less extreme case is the case of `ArrayUtilsTest#testShuffleBoolean` that shuffles an array that contains ten elements, but only two distinct values (true and false) has been seen to fail in Travis CI [1]. This patch removes this problematic assertion and leaves only the tests on the content of the array. [1] https://travis-ci.org/apache/commons-lang/jobs/346806985 You can merge this pull request into a Git repository by running: $ git pull https://github.com/mureinik/commons-lang shuffle-tests Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-lang/pull/316.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #316 commit fa1664f0abc5105e44e97fe8802b05965670dc67 Author: Allon Mureinik <amureini@...> Date: 2018-02-27T17:16:34Z Remove inequality check from shuffle tests The ArrayUtils.shuffle methods could conceivably shuffle an array in a way that leaves the shuffled array equal to the original array, and thus asserting that the shuffled array differs from the original array in the test only passes statistically. The chance of this happening increases as an inverse of the number of distinct values in the array. E.g., in the edge case of an array where all the elements are equal to each other, shuffling the array has a 100% chance of producing an array that's equal to the original array. A less extreme case is the case of ArrayUtilsTest#testShuffleBoolean that shuffles an array that contains ten elements, but only two distinct values (true and false) has been seen to fail in Travis CI [1]. This patch removes this problematic assertion and leaves only the tests on the content of the array. [1] https://travis-ci.org/apache/commons-lang/jobs/346806985 ---
[GitHub] commons-lang issue #301: Clean up EventUtilsTest
Github user mureinik commented on the issue: https://github.com/apache/commons-lang/pull/301 Changes since previous push: 1. Removed the patch regarding autoboxing, as per @britter's review 2. Rebased in order to consume the CI fixes merged to master ---
[GitHub] commons-lang issue #301: Clean up EventUtilsTest
Github user mureinik commented on the issue: https://github.com/apache/commons-lang/pull/301 The CI failure is unrelated to this PR, and also occurs on master. PR #302 should fix those errors, and once it's merged, I'll rebase this PR on top of it. ---
[GitHub] commons-lang pull request #302: Remove ObjectUtils' trailing white spaces
GitHub user mureinik opened a pull request: https://github.com/apache/commons-lang/pull/302 Remove ObjectUtils' trailing white spaces Commit 6ea2fc8 inadvertently introduced trailing white spaces in `ObjectUtils`' code, thus breaking the Checkstyle validation. This patch removes these redundant TWS in order to allow the build to pass. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mureinik/commons-lang ObjectUtils Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-lang/pull/302.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #302 commit f0930aa1512a408635d2286c01d49523b1234db7 Author: Allon Mureinik <amure...@redhat.com> Date: 2017-10-21T11:38:53Z Remove ObjectUtils' trailing white spaces Commit 6ea2fc8 inadvertently introduced trailing white spaces in ObjectUtils' code, thus breaking the Checkstyle validation. This patch removes these redundant TWS in order to allow the build to pass. ---
[GitHub] commons-lang pull request #301: Clean up EventUtilsTest
GitHub user mureinik opened a pull request: https://github.com/apache/commons-lang/pull/301 Clean up EventUtilsTest Clean up the `EventUtilsTest` class in order to make it easier to read and maintain, You can merge this pull request into a Git repository by running: $ git pull https://github.com/mureinik/commons-lang EventUtilsTest Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-lang/pull/301.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #301 commit 73d0c3617f392223b92e90f16365d7d2af649971 Author: Allon Mureinik <amure...@redhat.com> Date: 2017-10-21T10:29:06Z EventUtilsTest ExpectedException usage Use the ExpectedException @Rule to verify thrown exception instead of boiler-plate implementing its logic, in order to clean up the code and make it easier to read and maintain. commit 182e3aac0c12f1043497c156042ef2b9deb14a0c Author: Allon Mureinik <amure...@redhat.com> Date: 2017-10-21T10:31:42Z EventUtilsTest boxing Removed unnecessary explicit boxing and unboxing in order to make the code cleaner and easier to read. ---
[GitHub] commons-lang pull request #295: ExtendedMessageFormatTest integers
Github user mureinik closed the pull request at: https://github.com/apache/commons-lang/pull/295 ---
[GitHub] commons-lang issue #295: ExtendedMessageFormatTest integers
Github user mureinik commented on the issue: https://github.com/apache/commons-lang/pull/295 > Can you close the pull request? (I messed up the commit comment.) Done, thanks for merging! ---
[GitHub] commons-lang pull request #295: ExtendedMessageFormatTest integers
GitHub user mureinik opened a pull request: https://github.com/apache/commons-lang/pull/295 ExtendedMessageFormatTest integers Use the decimal "5" instead of the octal notation "05" to make the code more straight forward and easier to read. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mureinik/commons-lang ExtendedMessageFormatTest Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-lang/pull/295.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #295 commit e6b773af09df4f8300653cda13120a55b9056ca9 Author: Allon Mureinik <amure...@redhat.com> Date: 2017-10-05T18:46:26Z ExtendedMessageFormatTest integers Use the decimal "5" instead of the octal notation "05" to make the code more straight forward and easier to read. ---
[GitHub] commons-lang pull request #293: Char utils test
GitHub user mureinik opened a pull request: https://github.com/apache/commons-lang/pull/293 Char utils test Several enhancements to `CharUtilsTest` to make the test more robust and cover more functionality. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mureinik/commons-lang CharUtilsTest Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-lang/pull/293.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #293 commit c00920fe55dc61504bc0de2a8df004fc7264f5bf Author: Allon Mureinik <amure...@redhat.com> Date: 2017-09-30T12:25:34Z Clean up if in CharUtilsTest#testIsAscii_char The if statement calls assertTrue on the if branch and assertFalse on the else branch on the same expression. This can easily be simplified to assertEquals with a boolean expression to make the code clean and easier to read. commit 9512d67084b340c7493ff89f2f2cd2e4eee33241 Author: Allon Mureinik <amure...@redhat.com> Date: 2017-09-30T12:27:42Z CharUtilsTest#testIsAscii_char loop The loop currently loops only up to 128, thus testing just positive return values of CharUtils#isAscii(char). This patch increase the loop to go over all the possible values of an unsigned byte, thus testing also negative return values. commit b763decb69826b0fd4387e53dfd9c4ca5ab91467 Author: Allon Mureinik <amure...@redhat.com> Date: 2017-09-30T12:32:54Z Improve tests for CharUtils illegal usages CharUtilsTest has several instances of the following pattern: try { CharUtils.someMethod("illegal input"); } catch (final IllegalArgumentException ex) {} This pattern is not very useful for testing, as the test would pass whether an IllegalArgumentException is thrown or not. This patch enhances the test by explicitly failing it if the exception is not thrown: try { CharUtils.someMethod("illegal input"); fail("An IllegalArgumentException should have been thrown"); } catch (final IllegalArgumentException ex) {} ---
[GitHub] commons-lang pull request #289: Boolean comparisons in CharRange
GitHub user mureinik opened a pull request: https://github.com/apache/commons-lang/pull/289 Boolean comparisons in CharRange Cleaned up comparisons to false to just use the boolean negation operator (`!`). You can merge this pull request into a Git repository by running: $ git pull https://github.com/mureinik/commons-lang CharRange Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-lang/pull/289.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #289 commit ed86ae9f13d3d50a3641ed1af03032d71ffc5add Author: Allon Mureinik <amure...@redhat.com> Date: 2017-09-22T02:35:51Z Boolean comparisons in CharRange Cleaned up comparisons to false to just use the boolean negation operator (!). ---
[GitHub] commons-lang pull request #288: Fix typo stateme->statement
GitHub user mureinik opened a pull request: https://github.com/apache/commons-lang/pull/288 Fix typo stateme->statement You can merge this pull request into a Git repository by running: $ git pull https://github.com/mureinik/commons-lang findbugs Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-lang/pull/288.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #288 commit 0ff0122765e2a614b877cae3628c104c4d932a8f Author: Allon Mureinik <amure...@redhat.com> Date: 2017-09-21T10:28:09Z Fix typo stateme->statement ---
[GitHub] commons-lang pull request #287: Clean up findbugs exclusions for old XYZRang...
GitHub user mureinik opened a pull request: https://github.com/apache/commons-lang/pull/287 Clean up findbugs exclusions for old XYZRange classes Commit 56e830a removed all the old `XYZRange` classes from org.apache.lang3.math, and left only the generic `Range` class. This patch removes the left-over findbugs exclusions for those classes. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mureinik/commons-lang findbugs Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-lang/pull/287.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #287 commit f0c83f0d3440b9b6f918703dae918dc442ecb2c6 Author: Allon Mureinik <amure...@redhat.com> Date: 2017-09-20T16:52:03Z Clean up findbugs exclusions for old XYZRange classes Commit 56e830a removed all the old XYZRange classes from org.apache.lang3.math, and left only the generic Range class. This patch removes the left-over findbugs exclusions for those classes. ---
[GitHub] commons-lang issue #279: Fraction debug printings
Github user mureinik commented on the issue: https://github.com/apache/commons-lang/pull/279 How is this code useful? It adds nothing to the functionality, nor does it help explain the code. It seems like the original author used it while developing this class, but once it's done, I see no value in keeping it. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request #279: Fraction debug printings
GitHub user mureinik opened a pull request: https://github.com/apache/commons-lang/pull/279 Fraction debug printings Removed commented out debug printing from the Fraction class. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mureinik/commons-lang cleanup Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-lang/pull/279.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #279 commit 8d61cab5592e239d732aaeb19418db77970fda7f Author: Allon Mureinik <amure...@redhat.com> Date: 2017-07-22T10:02:47Z Fraction debug printings Removed commented out debug printing from the Fraction class. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request #274: Fix ExceptionUtils#getStackFrame(String) jav...
GitHub user mureinik opened a pull request: https://github.com/apache/commons-lang/pull/274 Fix ExceptionUtils#getStackFrame(String) javadoc `ExceptionUtils#getStackFrame(String)`'s javadoc contains a broken reference to `SystemUtils#LINE_SEPARATOR` (as the class name is not fully qualified, and there's no import of `org.apache.commons.lang3.SystemUtils`). This patch fixes the broken reference by replacing it it with a reference to `System#lineSeparator()`, which the method actually uses. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mureinik/commons-lang lineseparator Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-lang/pull/274.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #274 commit 564bd25adc51d29f991545395a5e53e0c96e3171 Author: Allon Mureinik <amure...@redhat.com> Date: 2017-07-01T11:50:41Z Fix ExceptionUtils#getStackFrame(String) javadoc ExceptionUtils#getStackFrame(String)'s javadoc contains a broken reference to SystemUtils#LINE_SEPARATOR (as the class name is not fully qualified, and there's no import of org.apache.commons.lang3.SystemUtils). This patch fixes the broken reference by replacing it it with a reference to System#lineSeparator(), which the method actually uses. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[jira] [Commented] (COLLECTIONS-601) commons-collections unit tests broken in Java 8
[ https://issues.apache.org/jira/browse/COLLECTIONS-601?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16024554#comment-16024554 ] Allon Mureinik commented on COLLECTIONS-601: I can confirm the build passes on my machine with the following environment: Apache Maven 3.3.9 (NON-CANONICAL_2016-07-01T11:53:38Z_mockbuild; 2016-07-01T14:53:38+03:00) Maven home: /usr/share/maven Java version: 1.8.0_131, vendor: Oracle Corporation Java home: /usr/lib/jvm/java-1.8.0-openjdk-1.8.0.131-1.b12.fc25.x86_64/jre Default locale: en_US, platform encoding: UTF-8 OS name: "linux", version: "4.10.15-200.fc25.x86_64", arch: "amd64", family: "unix" Thanks [~brunodepau...@yahoo.com.br] > commons-collections unit tests broken in Java 8 > --- > > Key: COLLECTIONS-601 > URL: https://issues.apache.org/jira/browse/COLLECTIONS-601 > Project: Commons Collections > Issue Type: Bug >Affects Versions: Nightly Builds > Environment: Maven 3.3.9, Java 1.8.0-121-1.b14, Fedora 25 - full > details in attached logs >Reporter: Allon Mureinik > Labels: test > Attachments: commons-collections.java7.build.log, > commons-collections.java8.build.log > > > Building the upstream trunk (currently, patch 30b2aca) fails the unit tests > in Java 8, but passes in Java 7 on the same machine. Attached logs with env > details an errors of a successful Java 7 build and an unsuccessful Java 8 > build -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[GitHub] commons-lang pull request #264: HashSetvBitSetTest diamond operator
GitHub user mureinik opened a pull request: https://github.com/apache/commons-lang/pull/264 HashSetvBitSetTest diamond operator Use Java 7's diamond operator to make the code a tad more elegant, as done in the rest of the codebase. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mureinik/commons-lang diamond Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-lang/pull/264.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #264 commit 8e891eb3599663f72499eae421e60f12025adc89 Author: Allon Mureinik <amure...@redhat.com> Date: 2017-04-29T07:37:40Z HashSetvBitSetTest diamond operator Use Java 7's diamond operator to make the code a tad more elegant, as done in the rest of the codebase. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request #263: Use String#contains where possible
GitHub user mureinik opened a pull request: https://github.com/apache/commons-lang/pull/263 Use String#contains where possible Since the project defines a JDK 7 source compatibility, it's safe to use JDK 5's features. This patch replaces usages of `String#indexOf` with `String#contains` where possible to make the code easier to read and maintain. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mureinik/commons-lang contains Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-lang/pull/263.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #263 commit 8fff6277fd22866753be3d2b3d717a32fba428a3 Author: Allon Mureinik <amure...@redhat.com> Date: 2017-04-22T19:59:38Z Use String#contains where possible Since the project defines a JDK 7 source compatibility, it's safe to use JDK 5's features. This patch replaces usages of String#indexOf with String#contains where possible to make the code easier to read and maintain. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request #262: Findbugs in Travis CI
GitHub user mureinik opened a pull request: https://github.com/apache/commons-lang/pull/262 Findbugs in Travis CI This PR adds FindBugs to Travis CI so it can be used to automatically evaluate new patches instead of having to do so retroactively. It contains a series of patches that either fix existing FindBugs errors or excludes them (either because the violation is intentional or because FindBugs cannot properly assess the code) - individual commit messages explain individual decisions. At the end of that series of patches is a patch that adds the `findbugs:check` goal to Travis CI, so it would now be applied on any new PR. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mureinik/commons-lang findbugs Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-lang/pull/262.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #262 commit a96cd5fd9ed9163101591eb83d965b425cbe211b Author: Allon Mureinik <amure...@redhat.com> Date: 2017-04-01T08:44:39Z FindBugs exclude filter for StringUtils Exclude ES_COMPARING_PARAMETER_STRING_WITH_EQ FindBugs warnings from StringUtils methods compare(String, String, boolean) and compareIgnoreCase(String, String, boolean). The usages of the == operator seem to be intentional optimizations similar to the usage in indexOfDifference. If this reasoning is ever overruled, this suppression should be removed. commit b4fa061c2991c3dc9832570b80ef55ce125b533d Author: Allon Mureinik <amure...@redhat.com> Date: 2017-04-01T09:18:53Z Exclude SF_SWITCH_NO_DEFAULT on FastDateParser FastDateParser#simpleQuote uses a switch case that actually has a default branch in it, but doesn't use break statements. SF_SWITCH_NO_DEFAULT unfortunately cannot recognize this pattern, and leave us with no choice but to suppress it. commit 44d3dddcd1c6f3161395489027b6963fa8e0f071 Author: Allon Mureinik <amure...@redhat.com> Date: 2017-04-01T09:38:54Z Exclude SF_SWITCH_FALLTHROUGH on FastDatePrinter FastDatePrinter#appendFullDigits uses a switch statement that intentionally falls through the cases. This patch adds a FindBugs suppression for it. commit 60e087c652f080d7826d1f244f1e6e9adc708e6a Author: Allon Mureinik <amure...@redhat.com> Date: 2017-04-01T09:18:53Z Exclude SF_SWITCH_NO_DEFAULT on FastDatePrinter FastDatePrinter#appendFullDigits uses a switch case without break statements. SF_SWITCH_NO_DEFAULT unfortunately cannot recognize this pattern, and leave us with no choice but to suppress it. commit 3525f756d087e3de370bc069fca96b7ab298b8c3 Author: Allon Mureinik <amure...@redhat.com> Date: 2017-04-01T09:16:52Z Add a default case to switch to appease FindBugs commit ea0b56f409699dbc5e983892d73315ed7139e452 Author: Allon Mureinik <amure...@redhat.com> Date: 2017-04-01T09:49:24Z Add FindBugs to Travis CI This patch copies the FindBugs configuration in pom.xml from the reporting section to the build section so findbugs can be used as part of the build process (by using the maven goal findbugs:check). It then adds this goal to the Travis CI build so that FindBugs becomes part of the CI, and new patches would be prevented from introducing new FindBugs errors. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[jira] [Updated] (COLLECTIONS-601) commons-collections unit tests broken in Java 8
[ https://issues.apache.org/jira/browse/COLLECTIONS-601?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Allon Mureinik updated COLLECTIONS-601: --- Attachment: (was: commons-collections.java7.build.log) > commons-collections unit tests broken in Java 8 > --- > > Key: COLLECTIONS-601 > URL: https://issues.apache.org/jira/browse/COLLECTIONS-601 > Project: Commons Collections > Issue Type: Bug >Affects Versions: Nightly Builds > Environment: Maven 3.3.9, Java 1.8.0-121-1.b14, Fedora 25 - full > details in attached logs > Reporter: Allon Mureinik > Labels: test > Attachments: commons-collections.java7.build.log, > commons-collections.java8.build.log > > > Building the upstream trunk (currently, patch 30b2aca) fails the unit tests > in Java 8, but passes in Java 7 on the same machine. Attached logs with env > details an errors of a successful Java 7 build and an unsuccessful Java 8 > build -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Updated] (COLLECTIONS-601) commons-collections unit tests broken in Java 8
[ https://issues.apache.org/jira/browse/COLLECTIONS-601?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Allon Mureinik updated COLLECTIONS-601: --- Attachment: (was: commons-collections.java8.build.log) > commons-collections unit tests broken in Java 8 > --- > > Key: COLLECTIONS-601 > URL: https://issues.apache.org/jira/browse/COLLECTIONS-601 > Project: Commons Collections > Issue Type: Bug >Affects Versions: Nightly Builds > Environment: Maven 3.3.9, Java 1.8.0-121-1.b14, Fedora 25 - full > details in attached logs > Reporter: Allon Mureinik > Labels: test > Attachments: commons-collections.java7.build.log, > commons-collections.java8.build.log > > > Building the upstream trunk (currently, patch 30b2aca) fails the unit tests > in Java 8, but passes in Java 7 on the same machine. Attached logs with env > details an errors of a successful Java 7 build and an unsuccessful Java 8 > build -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Updated] (COLLECTIONS-601) commons-collections unit tests broken in Java 8
[ https://issues.apache.org/jira/browse/COLLECTIONS-601?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Allon Mureinik updated COLLECTIONS-601: --- Attachment: commons-collections.java7.build.log commons-collections.java8.build.log > commons-collections unit tests broken in Java 8 > --- > > Key: COLLECTIONS-601 > URL: https://issues.apache.org/jira/browse/COLLECTIONS-601 > Project: Commons Collections > Issue Type: Bug >Affects Versions: Nightly Builds > Environment: Maven 3.3.9, Java 1.8.0-121-1.b14, Fedora 25 - full > details in attached logs > Reporter: Allon Mureinik > Labels: test > Attachments: commons-collections.java7.build.log, > commons-collections.java8.build.log > > > Building the upstream trunk (currently, patch 30b2aca) fails the unit tests > in Java 8, but passes in Java 7 on the same machine. Attached logs with env > details an errors of a successful Java 7 build and an unsuccessful Java 8 > build -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Updated] (COLLECTIONS-601) commons-collections unit tests broken in Java 8
[ https://issues.apache.org/jira/browse/COLLECTIONS-601?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Allon Mureinik updated COLLECTIONS-601: --- Attachment: (was: commons-collections.java8.build.log) > commons-collections unit tests broken in Java 8 > --- > > Key: COLLECTIONS-601 > URL: https://issues.apache.org/jira/browse/COLLECTIONS-601 > Project: Commons Collections > Issue Type: Bug >Affects Versions: Nightly Builds > Environment: Maven 3.3.9, Java 1.8.0-121-1.b14, Fedora 25 - full > details in attached logs > Reporter: Allon Mureinik > Labels: test > Attachments: commons-collections.java7.build.log, > commons-collections.java8.build.log > > > Building the upstream trunk (currently, patch 30b2aca) fails the unit tests > in Java 8, but passes in Java 7 on the same machine. Attached logs with env > details an errors of a successful Java 7 build and an unsuccessful Java 8 > build -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Updated] (COLLECTIONS-601) commons-collections unit tests broken in Java 8
[ https://issues.apache.org/jira/browse/COLLECTIONS-601?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Allon Mureinik updated COLLECTIONS-601: --- Attachment: commons-collections.java7.build.log > commons-collections unit tests broken in Java 8 > --- > > Key: COLLECTIONS-601 > URL: https://issues.apache.org/jira/browse/COLLECTIONS-601 > Project: Commons Collections > Issue Type: Bug >Affects Versions: Nightly Builds > Environment: Maven 3.3.9, Java 1.8.0-121-1.b14, Fedora 25 - full > details in attached logs > Reporter: Allon Mureinik > Labels: test > Attachments: commons-collections.java7.build.log, > commons-collections.java8.build.log > > > Building the upstream trunk (currently, patch 30b2aca) fails the unit tests > in Java 8, but passes in Java 7 on the same machine. Attached logs with env > details an errors of a successful Java 7 build and an unsuccessful Java 8 > build -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Updated] (COLLECTIONS-601) commons-collections unit tests broken in Java 8
[ https://issues.apache.org/jira/browse/COLLECTIONS-601?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Allon Mureinik updated COLLECTIONS-601: --- Attachment: commons-collections.java8.build.log > commons-collections unit tests broken in Java 8 > --- > > Key: COLLECTIONS-601 > URL: https://issues.apache.org/jira/browse/COLLECTIONS-601 > Project: Commons Collections > Issue Type: Bug >Affects Versions: Nightly Builds > Environment: Maven 3.3.9, Java 1.8.0-121-1.b14, Fedora 25 - full > details in attached logs > Reporter: Allon Mureinik > Labels: test > Attachments: commons-collections.java7.build.log, > commons-collections.java8.build.log > > > Building the upstream trunk (currently, patch 30b2aca) fails the unit tests > in Java 8, but passes in Java 7 on the same machine. Attached logs with env > details an errors of a successful Java 7 build and an unsuccessful Java 8 > build -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Issue Comment Deleted] (COLLECTIONS-601) commons-collections unit tests broken in Java 8
[ https://issues.apache.org/jira/browse/COLLECTIONS-601?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Allon Mureinik updated COLLECTIONS-601: --- Comment: was deleted (was: Unsuccessful build with Java 8) > commons-collections unit tests broken in Java 8 > --- > > Key: COLLECTIONS-601 > URL: https://issues.apache.org/jira/browse/COLLECTIONS-601 > Project: Commons Collections > Issue Type: Bug >Affects Versions: Nightly Builds > Environment: Maven 3.3.9, Java 1.8.0-121-1.b14, Fedora 25 - full > details in attached logs > Reporter: Allon Mureinik > Labels: test > Attachments: commons-collections.java7.build.log, > commons-collections.java8.build.log > > > Building the upstream trunk (currently, patch 30b2aca) fails the unit tests > in Java 8, but passes in Java 7 on the same machine. Attached logs with env > details an errors of a successful Java 7 build and an unsuccessful Java 8 > build -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Updated] (COLLECTIONS-601) commons-collections unit tests broken in Java 8
[ https://issues.apache.org/jira/browse/COLLECTIONS-601?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Allon Mureinik updated COLLECTIONS-601: --- Attachment: commons-collections.java8.build.log Unsuccessful build with Java 8 > commons-collections unit tests broken in Java 8 > --- > > Key: COLLECTIONS-601 > URL: https://issues.apache.org/jira/browse/COLLECTIONS-601 > Project: Commons Collections > Issue Type: Bug >Affects Versions: Nightly Builds > Environment: Maven 3.3.9, Java 1.8.0-121-1.b14, Fedora 25 - full > details in attached logs > Reporter: Allon Mureinik > Labels: test > Attachments: commons-collections.java8.build.log > > > Building the upstream trunk (currently, patch 30b2aca) fails the unit tests > in Java 8, but passes in Java 7 on the same machine. Attached logs with env > details an errors of a successful Java 7 build and an unsuccessful Java 8 > build -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Created] (COLLECTIONS-601) commons-collections unit tests broken in Java 8
Allon Mureinik created COLLECTIONS-601: -- Summary: commons-collections unit tests broken in Java 8 Key: COLLECTIONS-601 URL: https://issues.apache.org/jira/browse/COLLECTIONS-601 Project: Commons Collections Issue Type: Bug Affects Versions: Nightly Builds Environment: Maven 3.3.9, Java 1.8.0-121-1.b14, Fedora 25 - full details in attached logs Reporter: Allon Mureinik Building the upstream trunk (currently, patch 30b2aca) fails the unit tests in Java 8, but passes in Java 7 on the same machine. Attached logs with env details an errors of a successful Java 7 build and an unsuccessful Java 8 build -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[GitHub] commons-lang issue #257: Apply checkstyle to test sources
Github user mureinik commented on the issue: https://github.com/apache/commons-lang/pull/257 Updated `checkstyle-suppressions.xml` with the missing license that caused rat to fail in the CI. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request #257: Apply checkstyle to test sources
GitHub user mureinik opened a pull request: https://github.com/apache/commons-lang/pull/257 Apply checkstyle to test sources This enforces the code style defined by the checkstyle checks to the test sources too. This PR contains a series of patches that fix checkstyle violations, and a final patch the enables checkstyle checks on the test sources. In the cases where it would just add robustness and not improve the code's readability and maintainability (specifically - the javadoc checks), those checks are explicitly suppressed. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mureinik/commons-lang test-checkstyle Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-lang/pull/257.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #257 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang issue #254: Add checkstyle to the CI
Github user mureinik commented on the issue: https://github.com/apache/commons-lang/pull/254 CI seems to have been stuck for several hours. What am I missing here? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang issue #254: Add checkstyle to the CI
Github user mureinik commented on the issue: https://github.com/apache/commons-lang/pull/254 Latest push fixes the typo (thraed->thread) as per @kinow's review. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request #254: Add checkstyle to the CI
Github user mureinik commented on a diff in the pull request: https://github.com/apache/commons-lang/pull/254#discussion_r105530384 --- Diff: checkstyle.xml --- @@ -36,11 +36,9 @@ limitations under the License. - --- End diff -- Yes. I explained this in the relevant commit (a7d7e37d091bf1ba3674c6fc617466b284aaac71) , but perhaps I should add the explanation to the PR's comment too, to make it more obvious. New checkstyle versions removed this check, so we can no longer use it. Since checkstyle is not type-aware, there's really no way of performing this check properly - see https://github.com/checkstyle/checkstyle/issues/473 for the discussion about this removal. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request #254: Add checkstyle to the CI
Github user mureinik commented on a diff in the pull request: https://github.com/apache/commons-lang/pull/254#discussion_r105530325 --- Diff: src/main/java/org/apache/commons/lang3/concurrent/annotation/package-info.java --- @@ -0,0 +1,22 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +/** + * Provides annotations to document classes' thraed safety --- End diff -- Arg :-( Will fix, thanks for noticing! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request #254: Add checkstyle to the CI
GitHub user mureinik opened a pull request: https://github.com/apache/commons-lang/pull/254 Add checkstyle to the CI The Apache Commons Lang project uses checktyle to enforce its coding style, but this is only done as part of the reporting phase, when the site is generated. Thus, using checkstyle becomes somewhat of an opt-in procedure, depending on the vigilance of the developers to check the report, instead of being an actual enforcement against violating the codestyle, as evident by the fact that the current checkstyle report contains several hundred(!) such violations. This PR contains a series of patches to fix the existing checkstyle violations and a final patch that adds checkstyle to Travis CI, making it a gating check instead of an opt-in practice. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mureinik/commons-lang checkstyle Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-lang/pull/254.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #254 commit 13ebd5a955787ca91990a713c8cdf482aa72ca2a Author: Allon Mureinik <amure...@redhat.com> Date: 2017-03-05T10:05:23Z Apply JavaDoc checkstyle only to public methods The defacto style of the project requires Javadoc for public methods only, but the checkstyle Javadoc check defaults to requiring them even for private methods, generating 46 errors. This patch sets the checkstyle Javadoc check's scope to public to clean up the checkstyle report so it can be enabled in the CI. If we wish to reset the check to a laxer scope, the aforementioned errors should be fixed first. commit c1c21cd9a7d1888f054813a50ab6c6a3b0517292 Author: Allon Mureinik <amure...@redhat.com> Date: 2017-03-11T07:52:41Z Remove unused SystemUtils import from ExceptionUtils commit e14660f1c86acf8c2432593d9ad1a51108c2e83c Author: Allon Mureinik <amure...@redhat.com> Date: 2017-03-11T07:54:27Z Replace tabs with spaces in DateUtils commit a7d7e37d091bf1ba3674c6fc617466b284aaac71 Author: Allon Mureinik <amure...@redhat.com> Date: 2017-03-11T08:13:44Z Upgrade maven-checkstyle-plugin to 2.17 This patch upgrades maven-checkstyle-plugin to the latest available version, 2.17. This is done in order to consume a fix for checkstyle wrongfully reporting an error if the @return javadoc tag was used in an annotation type, as it is in Guarded (line 36). Note that checkstyle has removed the RedundantThrows check (see discussion at https://github.com/checkstyle/checkstyle/issues/473), so it was removed from the project's checkstyle.xml configuration. commit c407e34e7ecfde0e72bbbed1e5e8106792140d64 Author: Allon Mureinik <amure...@redhat.com> Date: 2017-03-11T08:49:22Z org.apache.commons.lang3.concurrent.annotation package-info Added package-info.java to the org.apache.commons.lang3.concurrent.annotation package to solve a checkstyle violation. commit 063e538595897a481f93d0a926a09081777eae4a Author: Allon Mureinik <amure...@redhat.com> Date: 2017-03-11T09:14:16Z Add checkstyle to Travis CI Currently, checkstyle is only run as part of the reporting phase, and it's up to the developer to check the report manually. This patch adds the checkstyle configuration to the build plugins so it can be used to check the code (as opposed to just generate a report of the failures) and adds it to Travis CI's configuration so every new patch will be automatically checked against it. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang issue #252: [LANG-1314] Fix javadoc creation on with Java 8
Github user mureinik commented on the issue: https://github.com/apache/commons-lang/pull/252 @PascalSchumacher no harm done. I'd rather have commons-lang's javadoc build properly than one patch more or less to my so-called name. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request #252: [LANG-1314] Fix javadoc creation on with Jav...
GitHub user mureinik opened a pull request: https://github.com/apache/commons-lang/pull/252 [LANG-1314] Fix javadoc creation on with Java 8 Java 8's javadoc seems to be stricter than Java 7's, and the current head current fails to generate javadoc (`mvn javadoc:javadoc`) with Java 8. This PR contains a series of fixes to the various classes' javadoc which are required for Java 8 (but would also improve the javadoc generated with earlier java versions) and then adds the `javadoc:javadoc` target to Travis CI so we can guarantee it won't break again. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mureinik/commons-lang javadoc Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-lang/pull/252.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #252 commit 972d29447d115224674154e0150d51bf9ccf6524 Author: Allon Mureinik <amure...@redhat.com> Date: 2017-03-05T09:50:34Z Add @param for in ArrayUtils#insert commit e4147a700f6c49b954a6f91dc8f2f1499777e92c Author: Allon Mureinik <amure...@redhat.com> Date: 2017-03-05T09:53:33Z Fix StirngUtils tags in javadoc The paragraph Whitespace is defined by {@link Character#isWhitespace(char)}. appears in several places in the javadoc (presumably, copy-pasted from the original one to the others). This is obviously a mistake, as a paragraph should start with , not with . This patch fixes all the occurrences of this paragraph to the proper form: Whitespace is defined by {@link Character#isWhitespace(char)}. commit 54eff5e8a55f591c1ae3fe31fd04dce663bbe4e1 Author: Allon Mureinik <amure...@redhat.com> Date: 2017-03-05T09:56:05Z Remove tag from Computable's javadoc The standard javadoc doclet does not allow self closing tags (such as ). This patch removes such a tag from Computable's javadoc, as it's redundant anyway, as it's only used to create spaces between two existing paragraphs. commit 632431f9433ce228407218dabef7b00fa8464ef4 Author: Allon Mureinik <amure...@redhat.com> Date: 2017-03-05T16:03:01Z Add javadoc creation to Travis CI --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[jira] [Created] (LANG-1314) javadoc creation broken with Java 8
Allon Mureinik created LANG-1314: Summary: javadoc creation broken with Java 8 Key: LANG-1314 URL: https://issues.apache.org/jira/browse/LANG-1314 Project: Commons Lang Issue Type: Bug Components: General Environment: amureini@mureinik ~ $ mvn -version Apache Maven 3.3.9 (NON-CANONICAL_2016-07-01T11:53:38Z_mockbuild; 2016-07-01T14:53:38+03:00) Maven home: /usr/share/maven Java version: 1.8.0_121, vendor: Oracle Corporation Java home: /usr/lib/jvm/java-1.8.0-openjdk-1.8.0.121-1.b14.fc25.x86_64/jre Default locale: en_US, platform encoding: UTF-8 OS name: "linux", version: "4.9.12-200.fc25.x86_64", arch: "amd64", family: "unix" Reporter: Allon Mureinik Priority: Minor Java 8's javadoc seems to be stricter than the Java 7 one. With the current HEAD (commit e5ed4ff), mvn javadoc:javadoc passes with Java 7, but fails on the same machine with Java 8 (environment described above) with the following errors: [ERROR] Failed to execute goal org.apache.maven.plugins:maven-javadoc-plugin:2.10.4:javadoc (default-cli) on project commons-lang3: An error has occurred in JavaDocs report generation: [ERROR] Exit code: 1 - /home/amureini/src/git/commons-lang/src/main/java/org/apache/commons/lang3/ArrayUtils.java:8421: warning: no @param for [ERROR] public static T[] insert(final int index, final T[] array, final T... values) { [ERROR] ^ [ERROR] /home/amureini/src/git/commons-lang/src/main/java/org/apache/commons/lang3/StringUtils.java:316: error: unexpected end tag: [ERROR] * Whitespace is defined by {@link Character#isWhitespace(char)}. [ERROR] ^ [ERROR] /home/amureini/src/git/commons-lang/src/main/java/org/apache/commons/lang3/StringUtils.java:316: error: unexpected end tag: [ERROR] * Whitespace is defined by {@link Character#isWhitespace(char)}. [ERROR] ^ [ERROR] /home/amureini/src/git/commons-lang/src/main/java/org/apache/commons/lang3/StringUtils.java:347: error: unexpected end tag: [ERROR] * Whitespace is defined by {@link Character#isWhitespace(char)}. [ERROR] ^ [ERROR] /home/amureini/src/git/commons-lang/src/main/java/org/apache/commons/lang3/StringUtils.java:347: error: unexpected end tag: [ERROR] * Whitespace is defined by {@link Character#isWhitespace(char)}. [ERROR] ^ [ERROR] /home/amureini/src/git/commons-lang/src/main/java/org/apache/commons/lang3/StringUtils.java:370: error: unexpected end tag: [ERROR] * Whitespace is defined by {@link Character#isWhitespace(char)}. [ERROR] ^ [ERROR] /home/amureini/src/git/commons-lang/src/main/java/org/apache/commons/lang3/StringUtils.java:370: error: unexpected end tag: [ERROR] * Whitespace is defined by {@link Character#isWhitespace(char)}. [ERROR] ^ [ERROR] /home/amureini/src/git/commons-lang/src/main/java/org/apache/commons/lang3/StringUtils.java:403: error: unexpected end tag: [ERROR] * Whitespace is defined by {@link Character#isWhitespace(char)}. [ERROR] ^ [ERROR] /home/amureini/src/git/commons-lang/src/main/java/org/apache/commons/lang3/StringUtils.java:403: error: unexpected end tag: [ERROR] * Whitespace is defined by {@link Character#isWhitespace(char)}. [ERROR] ^ [ERROR] /home/amureini/src/git/commons-lang/src/main/java/org/apache/commons/lang3/StringUtils.java:436: error: unexpected end tag: [ERROR] * Whitespace is defined by {@link Character#isWhitespace(char)}. [ERROR] ^ [ERROR] /home/amureini/src/git/commons-lang/src/main/java/org/apache/commons/lang3/StringUtils.java:436: error: unexpected end tag: [ERROR] * Whitespace is defined by {@link Character#isWhitespace(char)}. [ERROR] ^ [ERROR] /home/amureini/src/git/commons-lang/src/main/java/org/apache/commons/lang3/StringUtils.java:1946: error: unexpected end tag: [ERROR] * Whitespace is defined by {@link Character#isWhitespace(char)}. [ERROR] ^ [ERROR] /home/amureini/src/git/commons-lang/src/main/java/org/apache/commons/lang3/StringUtils.java:1946: error: unexpected end tag: [ERROR] * Whitespace is defined by {@link Character#isWhitespace(char)}. [ERROR] ^ [ERROR] /home/amureini/src/git/commons-lang/src/main/java/org/apache/commons/lang3/StringUtils.java:7092: error: unexpected end tag: [ERROR] * Whitespace is defined by {@link Character#isWhitespace(char)}. [ERROR] ^ [ERROR] /home/amureini/src/git/commons-lang/src/main/java/org/apache/commons/lang3/StringUtils.java:7092: error: unexpected end tag: [ERROR] * Whitespace is defined by {@link Character#isWhitespace(char)}. [ERROR] ^ [ERROR] /home/amureini/src/git/commons-lang/src/main/java/org/apache/commons/lang3/StringUtils.java:7241: error: unexpected end tag: [ERROR] * Whitespace is defined by {@link Character#isWhitespace(char)}. [ERROR] ^ [ERROR] /home/amureini/src/git/commons-lang/src/main/java/org/apache/commons/lang3/StringUtils.java:7241: error: unexpected end tag: [ERROR] * Whitespace is defined by {@li
[GitHub] commons-lang pull request #249: Checkstyle for long literals
GitHub user mureinik opened a pull request: https://github.com/apache/commons-lang/pull/249 Checkstyle for long literals PR #248 corrected all the long literals to use the upper case L notation. This patch finishes the job as per the discussion there, and adds a checktysle check to ensure no code that introduces long literals with lowercase l are introduced. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mureinik/commons-lang long-checkstyle Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-lang/pull/249.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #249 commit 380744a3ff0f8db9ea52260da90fbef825798407 Author: Allon Mureinik <amure...@redhat.com> Date: 2017-03-04T16:38:19Z Checkstyle for long literals PR #248 corrected all the long literals to use the upper case L notation. This patch finishes the job as per the discussion there, and adds a checktysle check to ensure no code that introduces long literals with lowercase l are introduced. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang issue #248: Longs
Github user mureinik commented on the issue: https://github.com/apache/commons-lang/pull/248 @PascalSchumacher I posed PR #249 to do so, thanks. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang issue #248: Longs
Github user mureinik commented on the issue: https://github.com/apache/commons-lang/pull/248 @PascalSchumacher Using uppercase L for long literals can also easily be enforced by checkstyle. Should I add this check, or would that be more of a nuisance than it's worth? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request #248: Longs
GitHub user mureinik opened a pull request: https://github.com/apache/commons-lang/pull/248 Longs Style improvement for handling long literals throughout the project. This patch contains: - Using long literals instead of casting int literals - Using uppercase L to signify long literals You can merge this pull request into a Git repository by running: $ git pull https://github.com/mureinik/commons-lang longs Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-lang/pull/248.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #248 commit 7e6c7ed701eb376a4cdf8ff263e8b62941d953a0 Author: Allon Mureinik <amure...@redhat.com> Date: 2017-03-04T03:55:03Z Use long literals This patch replaces int literals that were cast to longs (e.g., "(long) 1)" with long literals (e.g., "1L"), making the code cleaner and easier to maintain. commit 0dc8070af247bb0bbc240b533b0a73cb27277cf8 Author: Allon Mureinik <amure...@redhat.com> Date: 2017-03-04T03:59:58Z Use uppercase L for long literals Long literals can be specified by a lower case l (e.g., 1l) or by an uppercase one (e.g., 1L). This patch converts all existing long literals to use the uppercase L, as in some terminals, the lowercase l and the 1 characters are graphically similar, leading to possible confusions. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request #247: JUnit imports
GitHub user mureinik opened a pull request: https://github.com/apache/commons-lang/pull/247 JUnit imports The `junit.framework` package has been deprecated, and should no longer be used. This patch fixes the one remaining import from it to statically import `org.junit.Assert.assertNull` instead. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mureinik/commons-lang junit Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-lang/pull/247.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #247 commit 1aaa66937f6c16e0b5055354ab2bd595d679e9ae Author: Allon Mureinik <amure...@redhat.com> Date: 2017-02-28T21:17:51Z JUnit imports The junit.framework package has been deprecated, and should no longer be used. This patch fixes the one remaining import from it to statically import org.junit.Assert.assertNull instead. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request #246: DateUtilsTest asserts
GitHub user mureinik opened a pull request: https://github.com/apache/commons-lang/pull/246 DateUtilsTest asserts Use JUnit's assertFalse for assertions with conditions instead of re-implementing the logic here by testing the condition and throwing an AssertionFailureException if the condition is met. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mureinik/commons-lang DateUtilsTest Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-lang/pull/246.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #246 commit 59effed7ad53df5a59bee8b09b8e21ba9900156b Author: Allon Mureinik <amure...@redhat.com> Date: 2017-02-27T19:35:03Z DateUtilsTest asserts Use JUnit's assertFalse for assertions with conditions instead of re-implementing the logic here by testing the condition and throwing an AssertionFailureException if the condition is met. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang issue #245: Use foreach
Github user mureinik commented on the issue: https://github.com/apache/commons-lang/pull/245 @PascalSchumacher Missed that discussion somehow. I've reduced the scope of the patch to handle only for loops that go over arrays. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request #245: Use foreach
GitHub user mureinik opened a pull request: https://github.com/apache/commons-lang/pull/245 Use foreach Since the project no longer supports Java versions older than 6, it's safe to use the enhanced for loop syntax introduced in Java 5. This patch employs this syntax where possible to clean up the code. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mureinik/commons-lang foreach Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-lang/pull/245.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #245 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request #243: Fix JsonToStringStyleTest.NestingPerson java...
GitHub user mureinik opened a pull request: https://github.com/apache/commons-lang/pull/243 Fix JsonToStringStyleTest.NestingPerson javadoc A `{@link}` javadoc can only reference a resolvable class name. The `{@link}` tag in `NestingPerson`'s javadoc is thus broken, as it directly references `JsonToStringStyle` without qualifying it with the enclosing `ToStringStyle` class. This patch adds the enclosing class to the javadoc, thus "unbreaking" the reference. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mureinik/commons-lang javadoc Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-lang/pull/243.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #243 commit fb833014528684fd65a9712894e32120b32a261d Author: Allon Mureinik <amure...@redhat.com> Date: 2017-02-22T23:20:47Z Fix JsonToStringStyleTest.NestingPerson javadoc A {@link} javadoc can only reference a resolvable class name. The {@link} tag in NestingPerson's javadoc is thus broken, as it directly references JsonToStringStyle without qualifying it with the enclosing ToStringStyle class. This patch adds the enclosing class to the javadoc, thus "unbreaking" the reference. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request #242: Fix FastDateParser#getStrategy(char, int, Ca...
GitHub user mureinik opened a pull request: https://github.com/apache/commons-lang/pull/242 Fix FastDateParser#getStrategy(char, int, Calendar) javadoc The javadoc refers to a `formatField` parameter, which the method doesn't have. Reading the description and the method's code, this documentation clearly refers to the `f` parameter. This patch fixes the javadoc and aligns it with the method's parameters. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mureinik/commons-lang FastDateParser-javadoc Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-lang/pull/242.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #242 commit ce2a224f8e61ae9ceaa10de4e145ab95aa2f1170 Author: Allon Mureinik <amure...@redhat.com> Date: 2017-02-22T08:35:57Z Fix FastDateParser#getStrategy(char, int, Calendar) javadoc The javadoc refers to a formatField parameter, which the method doesn't have. Reading the description and the method's code, this documentation clearly refers to the f parameter. This patch fixes the javadoc and aligns it with the method's parameters. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request #240: Remove redundant semicolons from enums
Github user mureinik commented on a diff in the pull request: https://github.com/apache/commons-lang/pull/240#discussion_r102256442 --- Diff: src/test/java/org/apache/commons/lang3/EnumUtilsTest.java --- @@ -418,10 +418,10 @@ public void test_processBitVectors_longClass() { A00, A01, A02, A03, A04, A05, A06, A07, A08, A09, A10, A11, A12, A13, A14, A15, A16, A17, A18, A19, A20, A21, A22, A23, A24, A25, A26, A27, A28, A29, A30, A31, A32, A33, A34, A35, A36, A37, A38, A39, A40, A41, A42, A43, A44, A45, A46, A47, -A48, A49, A50, A51, A52, A53, A54, A55, A56, A57, A58, A59, A60, A61, A62, A63; +A48, A49, A50, A51, A52, A53, A54, A55, A56, A57, A58, A59, A60, A61, A62, A63 } enum TooMany { A,B,C,D,E,F,G,H,I,J,K,L,M,N,O,P,Q,R,S,T,U,V,W,X,Y,Z, A1,B1,C1,D1,E1,F1,G1,H1,I1,J1,K1,L1,M1,N1,O1,P1,Q1,R1,S1,T1,U1,V1,W1,X1,Y1,Z1, -A2,B2,C2,D2,E2,F2,G2,H2,I2,J2,K2,L2,M2; +A2,B2,C2,D2,E2,F2,G2,H2,I2,J2,K2,L2,M2 } --- End diff -- I kept the style the code had. If we collectively decide that spaces would make the code better, I'd gladly add them. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request #240: Remove redundant semicolons from enums
GitHub user mureinik opened a pull request: https://github.com/apache/commons-lang/pull/240 Remove redundant semicolons from enums While enums allow ending the member list in a semicolon(;), it's redundant. Some enums in the codebase use a semicolon in the end, and some do not. This patch standardizes the codebase's enum and cleans up the code by removing these semicolons. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mureinik/commons-lang enum-semicolon Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-lang/pull/240.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #240 commit c0d2d5b632211bb6ebc6fc0e93e133be1c3184ff Author: Allon Mureinik <amure...@redhat.com> Date: 2017-02-21T09:18:54Z Remove redundant semicolons from enums While enums allow ending the member list in a semicolon(;), it's redundant. Some enums in the codebase use a semicolon in the end, and some do not. This patch standardizes the codebase's enum and cleans up the code by removing these semicolons. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang issue #238: Validate's String.format without arguments
Github user mureinik commented on the issue: https://github.com/apache/commons-lang/pull/238 Now that I think about it, a cleaner solution might be to add `Object...` arguments to these methods' signatures, and pass it on to the `String.format` calls. This way, we keep the code 100% backwards compatible, regardless of any `%n`s anyone might have used - any existing calls just pass empty varargs, which is perfectly legal. The downside is slightly less elegant signatures. If the maintainers prefer this approach, I'd be happy to push such a patch. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang issue #238: Validate's String.format without arguments
Github user mureinik commented on the issue: https://github.com/apache/commons-lang/pull/238 Actually, it's worse - If your string contains any other formatter other than `%n` these methods would throw a `MissingFormatArgumentException`, as you have no way to pass the arguments to the formatted string. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request #238: Validate's String.format without arguments
GitHub user mureinik opened a pull request: https://github.com/apache/commons-lang/pull/238 Validate's String.format without arguments While calling `String.format("some string")` without any additional arguments is not technically wrong, it's redundant, as it just returns the same string. Removing these calls and just using the string instead both cleans up the code and offers a (very slight) performance gain. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mureinik/commons-lang string-format Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-lang/pull/238.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #238 commit 183c2522c5efce9f7d2e6f09e6fe872fd6ff0b8f Author: Allon Mureinik <amure...@redhat.com> Date: 2017-02-18T09:56:09Z Validate's String.format without arguments While calling String.format("some string") without any additional arguments is not technically wrong, it's redundant, as it just returns the same string. Removing these calls and just using the string instead both cleans up the code and offers a (very slight) performance gain. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request #236: Use StringBuilders instead of StringBuffers ...
GitHub user mureinik opened a pull request: https://github.com/apache/commons-lang/pull/236 Use StringBuilders instead of StringBuffers for local variables Some old parts of the code, originally written before Java 5 was available use StringBuffers in their local variables. Since local variables pose no concurrency problems, they can safely be replaced with non thread safe StringBuilders, for a slight performance gain. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mureinik/commons-lang StringBuilder Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-lang/pull/236.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #236 commit 2f6aa84a98a8e7a84049aa681ddda1feb2a40c7d Author: Allon Mureinik <amure...@redhat.com> Date: 2017-02-16T18:39:27Z Use StringBuilders instead of StringBuffers Some old parts of the code, originally written before Java 5 was available use StringBuffers in their local variables. Since local variables pose no concurrency problems, they can safely be replaced with non thread safe StringBuilders, for a slight performance gain. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request #234: ObjectUtilsTest: collapse empty catch blocks
GitHub user mureinik opened a pull request: https://github.com/apache/commons-lang/pull/234 ObjectUtilsTest: collapse empty catch blocks This patch employs Java 7's syntax to catch multiple exceptions in the same block with the | operator to make ObjectUtilsTest's code cleaner and easier to read. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mureinik/commons-lang ObjectUtilsTest Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-lang/pull/234.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #234 commit 07811434b773b6e1737fe7dd3775aeab8afb18b8 Author: Allon Mureinik <amure...@redhat.com> Date: 2017-02-11T19:35:16Z ObjectUtilsTest: collapse empty catch blocks This patch employs Java 7's syntax to catch multiple exceptions in the same block with the | operator to make ObjectUtilsTest's code cleaner and easier to read. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[jira] [Commented] (LANG-804) Redundant check for zero in HashCodeBuilder ctor
[ https://issues.apache.org/jira/browse/LANG-804?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13757502#comment-13757502 ] Allon Mureinik commented on LANG-804: - There are two issues here: The original issue - is there any point in explicitly checking (and documenting) for zero? Probably not, as you stated in the original case, and as my proposed patch solves. Should the ctor allow negative coefficients? With the given algorithm, the proper way to ensure that there are a minimal amount of clashes is to use two primes. Two relative primes could be also probably be used pretty safely. The question is how strict do you want to be? IMHO, disallowing negative coefficients is an arbitrary limitation. (-17, -37) would probably give better hash codes than (1, 1), or even any combination of the form (a, na) for most classes. Should those be disallowed too? (and if so - should/could this be done now? It would break backwards compatibility) Redundant check for zero in HashCodeBuilder ctor Key: LANG-804 URL: https://issues.apache.org/jira/browse/LANG-804 Project: Commons Lang Issue Type: Improvement Components: lang.builder.* Reporter: Sebb Priority: Minor The HashCodeBuilder(int, int) ctor checks both parameters for zero, as well as checking for an odd number. Zero is never odd, so the zero check could be eliminated. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (LANG-804) Redundant check for zero in HashCodeBuilder ctor
[ https://issues.apache.org/jira/browse/LANG-804?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13756951#comment-13756951 ] Allon Mureinik commented on LANG-804: - There's nothing wrong with using negative params to create negative hash codes. Integer.valueOf(-100).hashCode() gives -100, so if its good enough for the JDK, it should be good enough for commons-lang :-) I've created a pull request on GitHub to remove the redundant checks, for your evaluation: https://github.com/apache/commons-lang/pull/10 Redundant check for zero in HashCodeBuilder ctor Key: LANG-804 URL: https://issues.apache.org/jira/browse/LANG-804 Project: Commons Lang Issue Type: Improvement Components: lang.builder.* Reporter: Sebb Priority: Minor The HashCodeBuilder(int, int) ctor checks both parameters for zero, as well as checking for an odd number. Zero is never odd, so the zero check could be eliminated. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira