[jira] [Commented] (BCEL-343) Improve JUnit assertions

2020-10-07 Thread Allon Mureinik (Jira)


[ 
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

2020-10-07 Thread Allon Mureinik (Jira)


 [ 
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

2020-10-07 Thread Allon Mureinik (Jira)


[ 
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

2020-10-05 Thread Allon Mureinik (Jira)


[ 
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

2020-10-05 Thread Allon Mureinik (Jira)
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

2020-10-05 Thread Allon Mureinik (Jira)


 [ 
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

2020-10-03 Thread Allon Mureinik (Jira)


[ 
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

2020-10-03 Thread Allon Mureinik (Jira)
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

2019-10-06 Thread Allon Mureinik (Jira)


[ 
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

2019-10-06 Thread Allon Mureinik (Jira)
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

2019-10-05 Thread Allon Mureinik (Jira)


[ 
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

2019-10-05 Thread Allon Mureinik (Jira)
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

2019-09-30 Thread Allon Mureinik (Jira)


[ 
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

2019-09-30 Thread Allon Mureinik (Jira)
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

2018-10-13 Thread mureinik
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

2018-10-13 Thread mureinik
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

2018-10-11 Thread mureinik
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

2018-10-10 Thread Allon Mureinik (JIRA)


 [ 
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...

2018-10-10 Thread mureinik
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

2018-10-10 Thread Allon Mureinik (JIRA)
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

2018-10-10 Thread mureinik
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

2018-10-10 Thread mureinik
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

2018-10-09 Thread mureinik
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

2018-10-09 Thread mureinik
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

2018-10-09 Thread mureinik
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

2018-10-09 Thread mureinik
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

2018-10-08 Thread mureinik
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

2018-10-08 Thread mureinik
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

2018-10-07 Thread mureinik
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

2018-10-07 Thread mureinik
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

2018-10-06 Thread mureinik
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

2018-10-06 Thread mureinik
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

2018-10-06 Thread mureinik
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

2018-10-06 Thread mureinik
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

2018-10-04 Thread Allon Mureinik (JIRA)


[ 
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

2018-10-04 Thread mureinik
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

2018-10-04 Thread Allon Mureinik (JIRA)
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

2018-10-04 Thread mureinik
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

2018-10-04 Thread mureinik
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

2018-10-01 Thread mureinik
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

2018-04-03 Thread mureinik
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

2018-04-03 Thread mureinik
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

2018-02-27 Thread mureinik
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

2018-02-27 Thread mureinik
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

2017-10-21 Thread mureinik
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

2017-10-21 Thread mureinik
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

2017-10-21 Thread mureinik
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

2017-10-21 Thread mureinik
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

2017-10-07 Thread mureinik
Github user mureinik closed the pull request at:

https://github.com/apache/commons-lang/pull/295


---


[GitHub] commons-lang issue #295: ExtendedMessageFormatTest integers

2017-10-07 Thread mureinik
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

2017-10-06 Thread mureinik
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

2017-09-30 Thread mureinik
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

2017-09-21 Thread mureinik
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

2017-09-21 Thread mureinik
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...

2017-09-20 Thread mureinik
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

2017-07-22 Thread mureinik
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

2017-07-22 Thread mureinik
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...

2017-07-01 Thread mureinik
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

2017-05-25 Thread Allon Mureinik (JIRA)

[ 
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

2017-04-29 Thread mureinik
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

2017-04-22 Thread mureinik
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

2017-04-01 Thread mureinik
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

2017-03-20 Thread Allon Mureinik (JIRA)

 [ 
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

2017-03-20 Thread Allon Mureinik (JIRA)

 [ 
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

2017-03-20 Thread Allon Mureinik (JIRA)

 [ 
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

2017-03-20 Thread Allon Mureinik (JIRA)

 [ 
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

2017-03-20 Thread Allon Mureinik (JIRA)

 [ 
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

2017-03-20 Thread Allon Mureinik (JIRA)

 [ 
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

2017-03-20 Thread Allon Mureinik (JIRA)

 [ 
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

2017-03-20 Thread Allon Mureinik (JIRA)

 [ 
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

2017-03-20 Thread Allon Mureinik (JIRA)
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

2017-03-14 Thread mureinik
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

2017-03-14 Thread mureinik
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

2017-03-11 Thread mureinik
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

2017-03-11 Thread mureinik
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

2017-03-11 Thread mureinik
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

2017-03-11 Thread mureinik
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

2017-03-11 Thread mureinik
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

2017-03-06 Thread mureinik
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...

2017-03-05 Thread mureinik
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

2017-03-05 Thread Allon Mureinik (JIRA)
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

2017-03-04 Thread mureinik
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

2017-03-04 Thread mureinik
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

2017-03-04 Thread mureinik
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

2017-03-03 Thread mureinik
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

2017-02-28 Thread mureinik
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

2017-02-27 Thread mureinik
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

2017-02-24 Thread mureinik
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

2017-02-24 Thread mureinik
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...

2017-02-22 Thread mureinik
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...

2017-02-22 Thread mureinik
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

2017-02-21 Thread mureinik
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

2017-02-21 Thread mureinik
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

2017-02-20 Thread mureinik
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

2017-02-19 Thread mureinik
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

2017-02-19 Thread mureinik
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 ...

2017-02-16 Thread mureinik
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

2017-02-11 Thread mureinik
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

2013-09-04 Thread Allon Mureinik (JIRA)

[ 
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

2013-09-03 Thread Allon Mureinik (JIRA)

[ 
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


  1   2   >