[GitHub] commons-lang issue #182: Add maven dependency for JMH framework.

[kinow]

Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/182
  
Hi @C0rWin 

Some time ago I found this pull request and had to learn what was JMH :-) 
added a comment on 
[LANG-1110](https://issues.apache.org/jira/browse/LANG-1110), and today had 
time to revisit the issue and take a look at the replies.

So [Commons 
CSV](https://github.com/apache/commons-csv/blob/a7757849d3e9b2a37967ce208ab22f6d14567c98/pom.xml#L389)
 and [Commons 
RNG](https://github.com/apache/commons-rng/tree/688f3611131e05a072d998edb93e13492c3ed256/commons-rng-jmh)
 use JMH too. Both projects apply a similar approach, with the main difference 
being that Commons RNG - maybe for being a multi-module project - contains a 
jmh Maven module.

However, the dependencies and plug-in execution are part of a benchmark 
profile. Following the suggestions in 
[LANG-1110](https://issues.apache.org/jira/browse/LANG-1110), would be nice if 
we:

* Used a similar approach, such as profile, class names, etc
* Update [jmh dependencies to the 
latest](http://search.maven.org/#search%7Cga%7C1%7Cjmh-core)

I wanted to check with you before I opened a separate pull request. Would 
you be willing to update this pull request again with these changes? If so, 
just ping me and I'll reply here as soon as possible. Have plenty of time to 
work on Open Source projects in the next two months :-)

Cheers
Bruno


---
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 #182: Add maven dependency for JMH framework.

[C0rWin]

Github user C0rWin commented on the issue:

https://github.com/apache/commons-lang/pull/182
  
Hi Bruno,


On Sun, Feb 12, 2017 at 6:35 AM, Bruno P. Kinoshita <
notificati...@github.com> wrote:

> Hi @C0rWin 
>
> Some time ago I found this pull request and had to learn what was JMH :-)
> added a comment on LANG-1110
> , and today had time to
> revisit the issue and take a look at the replies.
>
> So Commons CSV
> 

> and Commons RNG
> 

> use JMH too. Both projects apply a similar approach, with the main
> difference being that Commons RNG - maybe for being a multi-module project
> - contains a jmh Maven module.
>
> However, the dependencies and plug-in execution are part of a benchmark
> profile. Following the suggestions in LANG-1110
> , would be nice if we:
>
>- Used a similar approach, such as profile, class names, etc
>- Update jmh dependencies to the latest
>
>
> I wanted to check with you before I opened a separate pull request. Would
> you be willing to update this pull request again with these changes? If 
so,
> just ping me and I'll reply here as soon as possible. Have plenty of time
> to work on Open Source projects in the next two months :-)
>

I can update my pull request to align it with according to your request.

Thanks,
Artem Barger.



---
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 #182: Add maven dependency for JMH framework.

[coveralls]

Github user coveralls commented on the issue:

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

[![Coverage 
Status](https://coveralls.io/builds/10113778/badge)](https://coveralls.io/builds/10113778)

Coverage remained the same at 94.536% when pulling 
**d38aea4c5b82f0bdd4800fa4153a55ba6e8fa4b9 on C0rWin:LANG-1256** into 
**b715d18f09591c510dded3a447a3ad1aacfa2173 on apache:master**.



---
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 #182: Add maven dependency for JMH framework.

[PascalSchumacher]

Github user PascalSchumacher commented on the issue:

https://github.com/apache/commons-lang/pull/182
  
I guess this is ready to merge?


---
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 #182: Add maven dependency for JMH framework.

[kinow]

Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/182
  
I was waiting for @C0rWin to update the pull request, to have an approach 
similar to what we have in other commons projects. If he has updated it, then 
+1 to merging (or maybe check with Emmanuel Bourg if it looks good too?)


---
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 #182: Add maven dependency for JMH framework.

[PascalSchumacher]

Github user PascalSchumacher commented on the issue:

https://github.com/apache/commons-lang/pull/182
  
As far as I can tell @C0rWin updated the pull request 9 days ago (after 
your conversation).

By the way merging this only makes sense if commons lang actually uses JHM. 
Do you (or somebody else) plan to work on this?


---
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 #182: Add maven dependency for JMH framework.

[kinow]

Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/182
  
Oh, missed that one, will take a look today to compare with [csv].

I've never seen it failing in Jenkins or Travis CI, but I suspect it is run 
manually, whenever someone needs to work on the classes involved in the 
benchmark tests for new features or debugging issues in different environments.


---
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 #182: Add maven dependency for JMH framework.

[kinow]

Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/182
  
Tried running the following to test the pull request:

```
$ mvn clean install
$ mvn test -Pbenchmark -e -X
```

And it kept failing with:

```
[INFO] BUILD FAILURE
[INFO] 

[INFO] Total time: 9.718 s
[INFO] Finished at: 2017-02-22T20:00:57+13:00
[INFO] Final Memory: 32M/486M
[INFO] 

[ERROR] Failed to execute goal 
org.codehaus.mojo:exec-maven-plugin:1.5.0:exec (benchmark) on project 
commons-lang3: Command execution failed. Process exited with an error: 1 (Exit 
value: 1) -> [Help 1]
org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute 
goal org.codehaus.mojo:exec-maven-plugin:1.5.0:exec (benchmark) on project 
commons-lang3: Command execution failed.
at 
org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:212)
at 
org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:153)
at 
org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:145)
at 
org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:116)
at 
org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:80)
at 
org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build(SingleThreadedBuilder.java:51)
at 
org.apache.maven.lifecycle.internal.LifecycleStarter.execute(LifecycleStarter.java:128)
at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:307)
at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:193)
at org.apache.maven.DefaultMaven.execute(DefaultMaven.java:106)
at org.apache.maven.cli.MavenCli.execute(MavenCli.java:863)
at org.apache.maven.cli.MavenCli.doMain(MavenCli.java:288)
at org.apache.maven.cli.MavenCli.main(MavenCli.java:199)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at 
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at 
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at 
org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced(Launcher.java:289)
at 
org.codehaus.plexus.classworlds.launcher.Launcher.launch(Launcher.java:229)
at 
org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode(Launcher.java:415)
at 
org.codehaus.plexus.classworlds.launcher.Launcher.main(Launcher.java:356)
Caused by: org.apache.maven.plugin.MojoExecutionException: Command 
execution failed.
at org.codehaus.mojo.exec.ExecMojo.execute(ExecMojo.java:302)
at 
org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo(DefaultBuildPluginManager.java:134)
at 
org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:207)
... 20 more
Caused by: org.apache.commons.exec.ExecuteException: Process exited with an 
error: 1 (Exit value: 1)
at 
org.apache.commons.exec.DefaultExecutor.executeInternal(DefaultExecutor.java:404)
at 
org.apache.commons.exec.DefaultExecutor.execute(DefaultExecutor.java:166)
at org.codehaus.mojo.exec.ExecMojo.executeCommandLine(ExecMojo.java:764)
at org.codehaus.mojo.exec.ExecMojo.executeCommandLine(ExecMojo.java:711)
at org.codehaus.mojo.exec.ExecMojo.execute(ExecMojo.java:289)
... 22 more
```

Tried playing with dependency versions and settings, using Commons CSV as 
example (which works for me, as long as manually install one maven dependency 
as per pom.xml comment) with no luck.

Will give it another try tomorrow at work.

```
$ mvn -v
Apache Maven 3.3.9 (bb52d8502b132ec0a5a3f4c09453c07478323dc5; 
2015-11-11T05:41:47+13:00)
Maven home: /opt/maven
Java version: 1.8.0_121, vendor: Oracle Corporation
Java home: /usr/lib/jvm/java-8-oracle/jre
Default locale: en_US, platform encoding: UTF-8
OS name: "linux", version: "4.4.0-62-generic", arch: "amd64", family: "unix"
```



---
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 #182: Add maven dependency for JMH framework.

[C0rWin]

Github user C0rWin commented on the issue:

https://github.com/apache/commons-lang/pull/182
  
I'd guess it fails since there is no JMH tests available in the project, so 
it fails to execute 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 issue #182: Add maven dependency for JMH framework.

[C0rWin]

Github user C0rWin commented on the issue:

https://github.com/apache/commons-lang/pull/182
  
Actually once this one will be in, I have a few tests based on JMH which I 
can push.


---
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 #182: Add maven dependency for JMH framework.

[kinow]

Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/182
  
@C0rWin what about trying to add the test from 
[LANG-1110](https://issues.apache.org/jira/browse/LANG-1110) to the pull 
request? Look at the attachments section.


---
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 #182: Add maven dependency for JMH framework.

[C0rWin]

Github user C0rWin commented on the issue:

https://github.com/apache/commons-lang/pull/182
  
I'm fine with it, will add the test from LANG-1110.


---
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 #182: Add maven dependency for JMH framework.

[PascalSchumacher]

Github user PascalSchumacher commented on the issue:

https://github.com/apache/commons-lang/pull/182
  
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 #182: Add maven dependency for JMH framework.

[coveralls]

Github user coveralls commented on the issue:

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

[![Coverage 
Status](https://coveralls.io/builds/7432683/badge)](https://coveralls.io/builds/7432683)

Coverage decreased (-0.02%) to 93.413% when pulling 
**9f4c0ec4197331687e410aca492d4071eded2462 on C0rWin:LANG-1256** into 
**648eebba22d6398a01b0924975bedced8469ac80 on apache:master**.



---
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 #182: Add maven dependency for JMH framework.

[C0rWin]

Github user C0rWin commented on the issue:

https://github.com/apache/commons-lang/pull/182
  
No Java code was involved in this PR, how that possible coverage reduced?


---
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 #182: Add maven dependency for JMH framework.

[PascalSchumacher]

Github user PascalSchumacher commented on the issue:

https://github.com/apache/commons-lang/pull/182
  
@C0rWin The OpenJDK 7 Build did not finish: 
https://travis-ci.org/apache/commons-lang/jobs/151969189 maybe that caused the 
change. Or there is a random element in the existing test?


---
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 #182: Add maven dependency for JMH framework.

[C0rWin]

Github user C0rWin commented on the issue:

https://github.com/apache/commons-lang/pull/182
  
@PascalSchumacher As far as I know coverage could be reduced only if I've 
added new code which is not tested or not covered by existing tests.


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