Bump qulice to 0.18.5 #886

victornoel · 2018-12-08T14:39:22Z

I had to change a test (RqMtFakeTest.closesAllParts) because of the rule AvoidFileStream that made me to use a different implementation for some stream and thus changed some of the exception thrown.

Apart from that, it is only cosmetics.

0crat · 2018-12-08T14:43:14Z

Job #886 is now in scope, role is REV

codecov-io · 2018-12-08T14:45:36Z

Codecov Report

Merging #886 into master will decrease coverage by 0.03%.
The diff coverage is 65.38%.

@@             Coverage Diff              @@
##             master     #886      +/-   ##
============================================
- Coverage     75.48%   75.44%   -0.04%     
+ Complexity      981      980       -1     
============================================
  Files           220      220              
  Lines          4724     4725       +1     
  Branches        369      372       +3     
============================================
- Hits           3566     3565       -1     
  Misses         1003     1003              
- Partials        155      157       +2

Impacted Files	Coverage Δ	Complexity Δ
src/main/java/org/takes/rq/RqGreedy.java	`87.5% <ø> (ø)`	`2 <0> (ø)`	⬇️
...ain/java/org/takes/facets/fallback/TkFallback.java	`50% <ø> (ø)`	`5 <0> (ø)`	⬇️
src/main/java/org/takes/rq/RqChunk.java	`75% <ø> (ø)`	`3 <0> (ø)`	⬇️
src/main/java/org/takes/rq/RqHeaders.java	`83.92% <ø> (ø)`	`0 <0> (ø)`	⬇️
src/main/java/org/takes/facets/auth/PsBasic.java	`85.71% <ø> (ø)`	`6 <0> (ø)`	⬇️
src/main/java/org/takes/rq/RqWithHeaders.java	`93.33% <ø> (ø)`	`3 <0> (ø)`	⬇️
src/main/java/org/takes/facets/auth/PsByFlag.java	`96.77% <ø> (ø)`	`13 <0> (ø)`	⬇️
src/main/java/org/takes/rq/RqHref.java	`66.66% <ø> (ø)`	`0 <0> (ø)`	⬇️
src/main/java/org/takes/rq/RqSimple.java	`66.66% <ø> (ø)`	`1 <0> (ø)`	⬇️
src/main/java/org/takes/rq/RqFake.java	`95.23% <ø> (ø)`	`8 <0> (ø)`	⬇️
... and 59 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 47ed1e8...cd1e411. Read the comment docs.

paulodamaso

@victornoel Please take a look

src/main/java/org/takes/facets/fork/FkHitRefresh.java

src/main/java/org/takes/http/MainRemote.java

src/main/java/org/takes/http/Options.java

src/main/java/org/takes/rs/Body.java

paulodamaso · 2018-12-10T17:11:05Z

src/test/java/org/takes/rq/multipart/RqMtFakeTest.java

-                ex.getMessage(),
-                Matchers.containsString(closed)
+                ex,
+                Matchers.instanceOf(ClosedChannelException.class)


@victornoel Let's use new IsInstanceOf instead Matchers.instanceOf

src/test/java/org/takes/rq/multipart/RqMtSmartTest.java

victornoel · 2018-12-10T17:59:21Z

@paulodamaso I didn't replace the new FileInputStream and co for fun ;)

It is caused by this PMD rule: https://pmd.github.io/latest/pmd_rules_java_performance.html#avoidfilestream

Maybe @krzyk can tell us more about the expected fix for this rule in qulice?

paulodamaso · 2018-12-10T18:03:06Z

@victornoel I see. Well, let's use cactoos' classes where possible then, to deal with these input and output streams

victornoel · 2018-12-10T18:16:24Z

@paulodamaso can you point to me which class should be used? If we end up using classes that wrap FileInputStream and such, is that a good or bad thing for quality? I would say bad :)

paulodamaso · 2018-12-10T19:04:11Z

@victornoel Why? I think that we should enforce and promote EO programming in takes, and officially we have cactoos with a lot of classes that do this file and input / outputstream work. Why not use them?

victornoel · 2018-12-10T19:11:32Z

@paulodamaso sorry, I guess I wasn't clear. I am 100% for EO style things. The question is: do we have actual classes that properly implements File-based InputStream as explained in the PMD link there: https://pmd.github.io/latest/pmd_rules_java_performance.html#avoidfilestream

If yes, then great, if not, it means we are just postponing solving this issue, because it means that the cactoos classes we would use are susceptible to the problems described in the PMD link.

If qulice integrated this rule, it means that it is important for quality to follow it.

Anyway, I'm ok with using any cactoos class there, I will make the change. In the end you are the one responsible as ARC, so it's good to me :P

paulodamaso · 2018-12-11T00:46:20Z

@victornoel You are right and I agree with you. I've checked cactoos and we do not have file-based inputstream access which prevents the problems that you showed on the link. Actually we use the "wrong way" there, and as cactoos qulice version is older too, this problems does not appear in quality checks for that project.

I've already notified @llorllale in cactoos project about this (yegor256/cactoos#891 (comment)), but we can't use this potentially unsafe code in our project. So I'll ask you to let all these file readers and input/output streams the way you made before (so they comply with qulice checks) and please put a puzzle somewhere so we do not forget about this and can fix this once cactoos updates its libraries. How about that?

victornoel · 2018-12-11T18:13:30Z

@paulodamaso I have updated the commits, thanks!

victornoel · 2018-12-11T18:14:42Z

@paulodamaso wait sorry, there is some conflicts to be resolved

victornoel · 2018-12-11T18:39:14Z

@paulodamaso this time it should be good :)

paulodamaso · 2018-12-11T18:46:23Z

@victornoel Almost there, please take a look at the comment I've made before: https://github.com/yegor256/takes/pull/886/files#r240299333

paulodamaso · 2018-12-11T19:44:37Z

@rultor merge

rultor · 2018-12-11T19:45:44Z

@rultor merge

@paulodamaso OK, I'll try to merge now. You can check the progress of the merge here

rultor · 2018-12-11T20:03:54Z

@rultor merge

@paulodamaso @victornoel Oops, I failed. You can see the full log here (spent 18min)

	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: An Ant BuildException has occured: apply returned: 255
around Ant part ...<apply failonerror="true" executable="xcop">... @ 10:51 in /home/r/repo/target/antrun/build-main.xml
	at org.apache.maven.plugin.antrun.AntRunMojo.execute(AntRunMojo.java:342)
	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: /home/r/repo/target/antrun/build-main.xml:10: apply returned: 255
	at org.apache.tools.ant.taskdefs.ExecTask.runExecute(ExecTask.java:643)
	at org.apache.tools.ant.taskdefs.ExecuteOn.runExec(ExecuteOn.java:410)
	at org.apache.tools.ant.taskdefs.ExecTask.execute(ExecTask.java:495)
	at org.apache.tools.ant.UnknownElement.execute(UnknownElement.java:292)
	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.apache.tools.ant.dispatch.DispatchUtils.execute(DispatchUtils.java:106)
	at org.apache.tools.ant.Task.perform(Task.java:348)
	at org.apache.tools.ant.taskdefs.Sequential.execute(Sequential.java:68)
	at net.sf.antcontrib.logic.IfTask.execute(IfTask.java:197)
	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.apache.tools.ant.dispatch.DispatchUtils.execute(DispatchUtils.java:106)
	at org.apache.tools.ant.TaskAdapter.execute(TaskAdapter.java:154)
	at org.apache.tools.ant.UnknownElement.execute(UnknownElement.java:292)
	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.apache.tools.ant.dispatch.DispatchUtils.execute(DispatchUtils.java:106)
	at org.apache.tools.ant.Task.perform(Task.java:348)
	at org.apache.tools.ant.Target.execute(Target.java:435)
	at org.apache.tools.ant.Target.performTasks(Target.java:456)
	at org.apache.tools.ant.Project.executeSortedTargets(Project.java:1393)
	at org.apache.tools.ant.Project.executeTarget(Project.java:1364)
	at org.apache.maven.plugin.antrun.AntRunMojo.execute(AntRunMojo.java:313)
	... 22 more
[ERROR] 
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException
container 8aace8568a1d270c4bc6ca6b851cb5828ad98eafc2d09ea733a5e810b60d68ae is dead
Tue Dec 11 21:03:45 CET 2018

victornoel · 2018-12-11T20:18:05Z

@paulodamaso I think my todo was properly formatted or something like that, I've pushed again

paulodamaso · 2018-12-11T20:30:23Z

@victornoel It's still failing, please take a look

paulodamaso · 2018-12-11T20:49:07Z

@rultor merge

rultor · 2018-12-11T20:50:50Z

@rultor merge

@paulodamaso OK, I'll try to merge now. You can check the progress of the merge here

rultor · 2018-12-11T20:59:00Z

@rultor merge

@paulodamaso @victornoel Oops, I failed. You can see the full log here (spent 8min)

++ curl -K ../curl-appveyor.cfg https://ci.appveyor.com/api/projects/yegor256/takes/build/3161
+ status=running
+ '[' running == null ']'
+ '[' running == success ']'
+ '[' running == failed ']'
+ echo 'waiting for AppVeyor build 3161: running'
waiting for AppVeyor build 3161: running
+ sleep 5s
+ true
++ jq -r .build.status
++ curl -K ../curl-appveyor.cfg https://ci.appveyor.com/api/projects/yegor256/takes/build/3161
+ status=running
+ '[' running == null ']'
+ '[' running == success ']'
+ '[' running == failed ']'
+ echo 'waiting for AppVeyor build 3161: running'
waiting for AppVeyor build 3161: running
+ sleep 5s
+ true
++ jq -r .build.status
++ curl -K ../curl-appveyor.cfg https://ci.appveyor.com/api/projects/yegor256/takes/build/3161
+ status=running
+ '[' running == null ']'
+ '[' running == success ']'
+ '[' running == failed ']'
+ echo 'waiting for AppVeyor build 3161: running'
waiting for AppVeyor build 3161: running
+ sleep 5s
+ true
++ curl -K ../curl-appveyor.cfg https://ci.appveyor.com/api/projects/yegor256/takes/build/3161
++ jq -r .build.status
+ status=running
+ '[' running == null ']'
+ '[' running == success ']'
+ '[' running == failed ']'
+ echo 'waiting for AppVeyor build 3161: running'
waiting for AppVeyor build 3161: running
+ sleep 5s
+ true
++ curl -K ../curl-appveyor.cfg https://ci.appveyor.com/api/projects/yegor256/takes/build/3161
++ jq -r .build.status
+ status=running
+ '[' running == null ']'
+ '[' running == success ']'
+ '[' running == failed ']'
+ echo 'waiting for AppVeyor build 3161: running'
waiting for AppVeyor build 3161: running
+ sleep 5s
+ true
++ jq -r .build.status
++ curl -K ../curl-appveyor.cfg https://ci.appveyor.com/api/projects/yegor256/takes/build/3161
+ status=failed
+ '[' failed == null ']'
+ '[' failed == success ']'
+ '[' failed == failed ']'
+ echo 'see https://ci.appveyor.com/project/yegor256/takes/build/3161'
see https://ci.appveyor.com/project/yegor256/takes/build/3161
+ exit 1
container 0b1cb4051a95faeaa13e87d2e0ebd449d7b567ae22ca64c33599cede44127eff is dead
Tue Dec 11 21:58:53 CET 2018

victornoel · 2018-12-12T08:00:15Z

@paulodamaso I'm not sure why there is a test not working in appveyor… strangely, I can't see the test being ran in the travis build.

victornoel · 2018-12-12T18:11:14Z

@paulodamaso I rebased on master since there were conflicts

paulodamaso · 2018-12-12T19:26:37Z

@rultor merge

rultor · 2018-12-12T19:28:46Z

@rultor merge

@paulodamaso OK, I'll try to merge now. You can check the progress of the merge here

rultor · 2018-12-12T19:52:02Z

@rultor merge

@paulodamaso Done! FYI, the full log is here (took me 23min)

0crat · 2018-12-12T20:06:49Z

Job gh:yegor256/takes#886 is not assigned, can't get performer

0crat · 2018-12-12T20:07:13Z

The job #886 is now out of scope

victornoel mentioned this pull request Dec 8, 2018

pom.xml:373-377: Upgrade qulice to latest 0.17.x and if... #880

Closed

0crat added the scope label Dec 8, 2018

paulodamaso suggested changes Dec 10, 2018

View reviewed changes

victornoel force-pushed the 880-qulice branch from a0d928c to 819ae28 Compare December 11, 2018 18:13

victornoel force-pushed the 880-qulice branch from 819ae28 to 765cdc3 Compare December 11, 2018 18:39

paulodamaso approved these changes Dec 11, 2018

View reviewed changes

victornoel force-pushed the 880-qulice branch from 765cdc3 to 73d692f Compare December 11, 2018 20:17

victornoel force-pushed the 880-qulice branch from 73d692f to 4e072c7 Compare December 11, 2018 20:41

Bump qulice to 0.18.5 for yegor256#880

a211201

Add todo to replace static methods with objects yegor256#880

cd1e411

victornoel force-pushed the 880-qulice branch from 4e072c7 to cd1e411 Compare December 12, 2018 18:11

rultor merged commit cd1e411 into yegor256:master Dec 12, 2018

victornoel deleted the 880-qulice branch December 12, 2018 19:57

0crat removed the scope label Dec 12, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bump qulice to 0.18.5 #886

Bump qulice to 0.18.5 #886

victornoel commented Dec 8, 2018

0crat commented Dec 8, 2018

codecov-io commented Dec 8, 2018 •

edited

paulodamaso left a comment

paulodamaso Dec 10, 2018

victornoel commented Dec 10, 2018

paulodamaso commented Dec 10, 2018

victornoel commented Dec 10, 2018

paulodamaso commented Dec 10, 2018

victornoel commented Dec 10, 2018

paulodamaso commented Dec 11, 2018

victornoel commented Dec 11, 2018

victornoel commented Dec 11, 2018

victornoel commented Dec 11, 2018

paulodamaso commented Dec 11, 2018

paulodamaso commented Dec 11, 2018

rultor commented Dec 11, 2018

rultor commented Dec 11, 2018

victornoel commented Dec 11, 2018

paulodamaso commented Dec 11, 2018

paulodamaso commented Dec 11, 2018

rultor commented Dec 11, 2018

rultor commented Dec 11, 2018

victornoel commented Dec 12, 2018

victornoel commented Dec 12, 2018

paulodamaso commented Dec 12, 2018

rultor commented Dec 12, 2018

rultor commented Dec 12, 2018

0crat commented Dec 12, 2018

0crat commented Dec 12, 2018

Bump qulice to 0.18.5 #886

Bump qulice to 0.18.5 #886

Conversation

victornoel commented Dec 8, 2018

0crat commented Dec 8, 2018

codecov-io commented Dec 8, 2018 • edited

Codecov Report

paulodamaso left a comment

Choose a reason for hiding this comment

paulodamaso Dec 10, 2018

Choose a reason for hiding this comment

victornoel commented Dec 10, 2018

paulodamaso commented Dec 10, 2018

victornoel commented Dec 10, 2018

paulodamaso commented Dec 10, 2018

victornoel commented Dec 10, 2018

paulodamaso commented Dec 11, 2018

victornoel commented Dec 11, 2018

victornoel commented Dec 11, 2018

victornoel commented Dec 11, 2018

paulodamaso commented Dec 11, 2018

paulodamaso commented Dec 11, 2018

rultor commented Dec 11, 2018

rultor commented Dec 11, 2018

victornoel commented Dec 11, 2018

paulodamaso commented Dec 11, 2018

paulodamaso commented Dec 11, 2018

rultor commented Dec 11, 2018

rultor commented Dec 11, 2018

victornoel commented Dec 12, 2018

victornoel commented Dec 12, 2018

paulodamaso commented Dec 12, 2018

rultor commented Dec 12, 2018

rultor commented Dec 12, 2018

0crat commented Dec 12, 2018

0crat commented Dec 12, 2018

codecov-io commented Dec 8, 2018 •

edited