search

yegor256 / cactoos-http

Object-Oriented HTTP Client
MIT License
37 stars 28 forks source link

Start work on having HtWire not close the socket #76

Closed victornoel closed 5 years ago

victornoel commented 5 years ago

This is for #63.

I introduced a test for the feature and added a new @todo to continue work. I had to refactor a bit some things around to make the test easier to write/read.

0crat commented 5 years ago

Job #76 is now in scope, role is REV

0crat commented 5 years ago

This pull request #76 is assigned to @golyalpha/z, here is why; the budget is 15 minutes, see §4; please, read §27 and when you decide to accept the changes, inform @llorllale/z (the architect) right in this ticket; if you decide that this PR should not be accepted ever, also inform the architect; this blog post will help you understand what is expected from a code reviewer; there will be no monetary reward for this job

victornoel commented 5 years ago

@golyalpha are you talking to me? (see https://www.yegor256.com/2014/11/24/principles-of-bug-tracking.html#4-avoid-noiseaddress-your-comments)

victornoel commented 5 years ago

@golyalpha also I think you are confusing the @todo I added and the @todo I removed.

Edit: actually I forgot to remove the other todo :) done

golyalpha commented 5 years ago

@victornoel Thanks for clearing that up. The todo is strikingly simmilar to the original one, except for the added "unignore the test", that's what confused me. I see you also removed the original @todo puzzle.

victornoel commented 5 years ago

The todo is strikingly simmilar to the original one, except for the added "unignore the test", that's what confused me. I see you also removed the original @todo puzzle.

@golyalpha yes, is this a problem?

codecov-io commented 5 years ago

Codecov Report

Merging #76 into master will decrease coverage by 0.02%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master   #76      +/-   ##
=========================================
- Coverage     95.02%   95%   -0.03%     
- Complexity       68    69       +1     
=========================================
  Files            17    17              
  Lines           181   180       -1     
  Branches         20    20              
=========================================
- Hits            172   171       -1     
  Misses            4     4              
  Partials          5     5
Impacted Files Coverage Δ Complexity Δ
...ain/java/org/cactoos/http/HtKeepAliveResponse.java 100% <ø> (ø) 2 <0> (ø) :arrow_down:
src/main/java/org/cactoos/http/HtResponse.java 72.72% <ø> (ø) 3 <0> (ø) :arrow_down:
src/main/java/org/cactoos/http/InputEnvelope.java 100% <100%> (ø) 2 <1> (?)
src/main/java/org/cactoos/http/HtWire.java 92.85% <100%> (-0.25%) 10 <2> (+1)

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 f7cf70c...52e8b09. Read the comment docs.

golyalpha commented 5 years ago

@llorllale At this point, this PR looks alright to merge to me.

golyalpha commented 5 years ago

The todo is strikingly simmilar to the original one, except for the added "unignore the test", that's what confused me. I see you also removed the original @todo puzzle.

@golyalpha yes, is this a problem?

@victornoel No, actually, I was in the middle of finishing up my review.

victornoel commented 5 years ago

@golyalpha hehe ok :) I like to use the review functionality of github so that I can write all my comments on the code and submit them all at once with a general comment at the end

victornoel commented 5 years ago

@llorllale ping?

llorllale commented 5 years ago

@victornoel left a couple of comments

victornoel commented 5 years ago

@llorllale I've taken into account one of the comment and answered the others

llorllale commented 5 years ago

@rultor merge

rultor commented 5 years ago

@rultor merge

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

rultor commented 5 years ago

@rultor merge

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

Downloading: https://repo.maven.apache.org/maven2/org/apache/commons/commons-lang3/3.4/commons-lang3-3.4.jar
20/20 KB   24/44 KB   229/229 KB   

Downloaded: https://repo.maven.apache.org/maven2/commons-beanutils/commons-beanutils/1.9.2/commons-beanutils-1.9.2.jar (229 KB at 3216.6 KB/sec)
Downloading: https://repo.maven.apache.org/maven2/org/hibernate/hibernate-validator/5.0.0.Final/hibernate-validator-5.0.0.Final.jar
20/20 KB   27/44 KB                
20/20 KB   29/44 KB   
20/20 KB   32/44 KB   
3/391 KB   20/20 KB   32/44 KB   
5/391 KB   20/20 KB   32/44 KB   
8/391 KB   20/20 KB   32/44 KB   
8/391 KB   20/20 KB   36/44 KB   
11/391 KB   20/20 KB   36/44 KB   
11/391 KB   20/20 KB   40/44 KB   
11/391 KB   20/20 KB   44/44 KB   
11/391 KB   20/20 KB   44/44 KB   
13/391 KB   20/20 KB   44/44 KB   
16/391 KB   20/20 KB   44/44 KB   
19/391 KB   20/20 KB   44/44 KB   
21/391 KB   20/20 KB   44/44 KB   
24/391 KB   20/20 KB   44/44 KB   

Downloaded: https://repo.maven.apache.org/maven2/org/sonatype/sisu/sisu-inject-plexus/2.6.0/sisu-inject-plexus-2.6.0.jar (20 KB at 256.6 KB/sec)
Downloading: https://repo.maven.apache.org/maven2/org/jboss/logging/jboss-logging/3.1.1.GA/jboss-logging-3.1.1.GA.jar
27/391 KB   44/44 KB              
29/391 KB   44/44 KB   
32/391 KB   44/44 KB   
36/391 KB   44/44 KB   
40/391 KB   44/44 KB   
44/391 KB   44/44 KB   
48/391 KB   44/44 KB   

Downloaded: https://repo.maven.apache.org/maven2/org/apache/commons/commons-lang3/3.4/commons-lang3-3.4.jar (0 B at 0.0 KB/sec)
52/391 KB   44/44 KB   
56/391 KB   44/44 KB   
60/391 KB   44/44 KB   
64/391 KB   44/44 KB   
68/391 KB   44/44 KB   
72/391 KB   44/44 KB   
76/391 KB   44/44 KB   
80/391 KB   44/44 KB   
84/391 KB   44/44 KB   
88/391 KB   44/44 KB   
92/391 KB   44/44 KB   
96/391 KB   44/44 KB   
100/391 KB   44/44 KB   
104/391 KB   44/44 KB   
108/391 KB   44/44 KB   
112/391 KB   44/44 KB   

Downloaded: https://repo.maven.apache.org/maven2/org/sonatype/sisu/sisu-inject-bean/2.6.0/sisu-inject-bean-2.6.0.jar (44 KB at 512.2 KB/sec)
116/391 KB              
120/391 KB   
124/391 KB   
128/391 KB   
132/391 KB   
136/391 KB   
140/391 KB   
144/391 KB   
144/391 KB   3/60 KB   
148/391 KB   3/60 KB   
152/391 KB   3/60 KB   
156/391 KB   3/60 KB   
160/391 KB   3/60 KB   
160/391 KB   5/60 KB   
160/391 KB   4/560 KB   5/60 KB   
160/391 KB   8/560 KB   5/60 KB   
160/391 KB   12/560 KB   5/60 KB   
160/391 KB   16/560 KB   5/60 KB   
160/391 KB   16/560 KB   8/60 KB   
160/391 KB   16/560 KB   11/60 KB   
160/391 KB   16/560 KB   13/60 KB   
164/391 KB   16/560 KB   13/60 KB   
168/391 KB   16/560 KB   13/60 KB   
172/391 KB   16/560 KB   13/60 KB   
176/391 KB   16/560 KB   13/60 KB   
176/391 KB   16/560 KB   16/60 KB   
176/391 KB   20/560 KB   16/60 KB   
176/391 KB   24/560 KB   16/60 KB   
176/391 KB   28/560 KB   16/60 KB   
176/391 KB   32/560 KB   16/60 KB   
180/391 KB   32/560 KB   16/60 KB   
184/391 KB   32/560 KB   16/60 KB   
188/391 KB   32/560 KB   16/60 KB   
192/391 KB   32/560 KB   16/60 KB   
192/391 KB   32/560 KB   19/60 KB   
192/391 KB   32/560 KB   21/60 KB   
192/391 KB   32/560 KB   24/60 KB   
192/391 KB   32/560 KB   27/60 KB   
196/391 KB   32/560 KB   27/60 KB   
200/391 KB   32/560 KB   27/60 KB   
204/391 KB   32/560 KB   27/60 KB   
208/391 KB   32/560 KB   27/60 KB   
208/391 KB   32/560 KB   29/60 KB   
208/391 KB   32/560 KB   32/60 KB   
212/391 KB   32/560 KB   32/60 KB   
216/391 KB   32/560 KB   32/60 KB   
220/391 KB   32/560 KB   32/60 KB   
224/391 KB   32/560 KB   32/60 KB   
228/391 KB   32/560 KB   32/60 KB   
232/391 KB   32/560 KB   32/60 KB   
236/391 KB   32/560 KB   32/60 KB   
240/391 KB   32/560 KB   32/60 KB   
240/391 KB   36/560 KB   32/60 KB   
240/391 KB   40/560 KB   32/60 KB   
240/391 KB   44/560 KB   32/60 KB   
240/391 KB   48/560 KB   32/60 KB   
244/391 KB   48/560 KB   32/60 KB   
248/391 KB   48/560 KB   32/60 KB   
252/391 KB   48/560 KB   32/60 KB   
256/391 KB   48/560 KB   32/60 KB   
256/391 KB   48/560 KB   36/60 KB   
256/391 KB   48/560 KB   40/60 KB   
256/391 KB   48/560 KB   44/60 KB   
256/391 KB   48/560 KB   48/60 KB   
256/391 KB   48/560 KB   52/60 KB   
256/391 KB   48/560 KB   56/60 KB   
260/391 KB   48/560 KB   56/60 KB   
260/391 KB   48/560 KB   60/60 KB   
264/391 KB   48/560 KB   60/60 KB   
268/391 KB   48/560 KB   60/60 KB   
272/391 KB   48/560 KB   60/60 KB   
272/391 KB   52/560 KB   60/60 KB   
272/391 KB   56/560 KB   60/60 KB   
272/391 KB   60/560 KB   60/60 KB   
272/391 KB   64/560 KB   60/60 KB   
276/391 KB   64/560 KB   60/60 KB   
280/391 KB   64/560 KB   60/60 KB   
284/391 KB   64/560 KB   60/60 KB   
288/391 KB   64/560 KB   60/60 KB   
288/391 KB   68/560 KB   60/60 KB   
288/391 KB   72/560 KB   60/60 KB   
288/391 KB   76/560 KB   60/60 KB   
288/391 KB   80/560 KB   60/60 KB   
292/391 KB   80/560 KB   60/60 KB   
296/391 KB   80/560 KB   60/60 KB   
300/391 KB   80/560 KB   60/60 KB   
304/391 KB   80/560 KB   60/60 KB   
308/391 KB   80/560 KB   60/60 KB   
312/391 KB   80/560 KB   60/60 KB   
316/391 KB   80/560 KB   60/60 KB   
320/391 KB   80/560 KB   60/60 KB   
324/391 KB   80/560 KB   60/60 KB   
328/391 KB   80/560 KB   60/60 KB   
332/391 KB   80/560 KB   60/60 KB   
336/391 KB   80/560 KB   60/60 KB   
336/391 KB   84/560 KB   60/60 KB   
336/391 KB   88/560 KB   60/60 KB   
336/391 KB   92/560 KB   60/60 KB   
336/391 KB   96/560 KB   60/60 KB   
340/391 KB   96/560 KB   60/60 KB   
344/391 KB   96/560 KB   60/60 KB   
348/391 KB   96/560 KB   60/60 KB   
352/391 KB   96/560 KB   60/60 KB   
356/391 KB   96/560 KB   60/60 KB   
360/391 KB   96/560 KB   60/60 KB   
364/391 KB   96/560 KB   60/60 KB   
368/391 KB   96/560 KB   60/60 KB   
368/391 KB   100/560 KB   60/60 KB   
368/391 KB   104/560 KB   60/60 KB   
368/391 KB   108/560 KB   60/60 KB   
368/391 KB   112/560 KB   60/60 KB   
372/391 KB   112/560 KB   60/60 KB   
376/391 KB   112/560 KB   60/60 KB   

380/391 KB   112/560 KB              
Downloaded: https://repo.maven.apache.org/maven2/org/jboss/logging/jboss-logging/3.1.1.GA/jboss-logging-3.1.1.GA.jar (60 KB at 478.8 KB/sec)
384/391 KB   112/560 KB   
388/391 KB   112/560 KB   
391/391 KB   112/560 KB   
391/391 KB   116/560 KB   
391/391 KB   120/560 KB   
391/391 KB   124/560 KB   
391/391 KB   128/560 KB   
391/391 KB   132/560 KB   
391/391 KB   136/560 KB   
391/391 KB   140/560 KB   
391/391 KB   144/560 KB   
391/391 KB   148/560 KB   
391/391 KB   152/560 KB   
391/391 KB   156/560 KB   
391/391 KB   160/560 KB   

Downloaded: https://repo.maven.apache.org/maven2/org/sonatype/sisu/sisu-guice/3.2.5/sisu-guice-3.2.5-no_aop.jar (391 KB at 2891.1 KB/sec)
164/560 KB                
168/560 KB   
172/560 KB   
176/560 KB   
180/560 KB   
184/560 KB   
188/560 KB   
192/560 KB   
196/560 KB   
200/560 KB   
204/560 KB   
208/560 KB   
212/560 KB   
216/560 KB   
220/560 KB   
224/560 KB   
228/560 KB   
232/560 KB   
236/560 KB   
240/560 KB   
244/560 KB   
248/560 KB   
252/560 KB   
256/560 KB   
260/560 KB   
264/560 KB   
268/560 KB   
272/560 KB   
276/560 KB   
280/560 KB   
284/560 KB   
288/560 KB   
292/560 KB   
296/560 KB   
300/560 KB   
304/560 KB   
308/560 KB   
312/560 KB   
316/560 KB   
320/560 KB   
324/560 KB   
328/560 KB   
332/560 KB   
336/560 KB   
340/560 KB   
344/560 KB   
348/560 KB   
352/560 KB   
356/560 KB   
360/560 KB   
364/560 KB   
368/560 KB   
372/560 KB   
376/560 KB   
380/560 KB   
384/560 KB   
388/560 KB   
392/560 KB   
396/560 KB   
400/560 KB   
404/560 KB   
408/560 KB   
412/560 KB   
416/560 KB   
420/560 KB   
424/560 KB   
428/560 KB   
432/560 KB   
436/560 KB   
440/560 KB   
444/560 KB   
448/560 KB   
452/560 KB   
456/560 KB   
460/560 KB   
464/560 KB   
468/560 KB   
472/560 KB   
476/560 KB   
480/560 KB   
484/560 KB   
488/560 KB   
492/560 KB   
496/560 KB   
500/560 KB   
504/560 KB   
508/560 KB   
512/560 KB   
516/560 KB   
520/560 KB   
524/560 KB   
528/560 KB   
532/560 KB   
536/560 KB   
540/560 KB   
544/560 KB   
548/560 KB   
552/560 KB   
556/560 KB   
560/560 KB   

Downloaded: https://repo.maven.apache.org/maven2/org/hibernate/hibernate-validator/5.0.0.Final/hibernate-validator-5.0.0.Final.jar (560 KB at 2613.9 KB/sec)
[INFO] Checkstyle: src/test/java/org/cactoos/http/HtWireTest.java[39]: Unused import - org.hamcrest.Matchers. (UnusedImportsCheck)
[INFO] PMD: src/test/java/org/cactoos/http/HtWireTest.java[39-39]: Avoid unused imports such as 'org.hamcrest.Matchers' (UnusedImports)
[INFO] Read our quality policy: http://www.qulice.com/quality.html
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 04:18 min
[INFO] Finished at: 2019-03-30T13:40:10+00:00
[INFO] Final Memory: 44M/318M
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal com.qulice:qulice-maven-plugin:0.17.3:check (jcabi-check) on project cactoos-http: Failure: There are 2 violations -> [Help 1]
org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal com.qulice:qulice-maven-plugin:0.17.3:check (jcabi-check) on project cactoos-http: Failure
    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.MojoFailureException: Failure
    at com.qulice.maven.CheckMojo.doExecute(CheckMojo.java:72)
    at com.qulice.maven.AbstractQuliceMojo.execute(AbstractQuliceMojo.java:172)
    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: com.qulice.spi.ValidationException: There are 2 violations
    at com.qulice.maven.CheckMojo.run(CheckMojo.java:116)
    at com.qulice.maven.CheckMojo.doExecute(CheckMojo.java:66)
    ... 23 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/MojoFailureException
container 987115369e4316257d000990ddff1a61e58ff40f260e05c5683e65b2cc85e23f is dead
Sat Mar 30 14:41:00 CET 2019
victornoel commented 5 years ago

@llorllale I have fixed the qulice error, sorry about that

llorllale commented 5 years ago

@rultor merge

rultor commented 5 years ago

@rultor merge

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

rultor commented 5 years ago

@rultor merge

@llorllale Done! FYI, the full log is here (took me 9min)

0crat commented 5 years ago

@sereshqua/z please review this job completed by @golyalpha/z, as in §30; the job will be fully closed and all payments will be made when the quality review is completed

0crat commented 5 years ago

Job #76 is not in the agenda of @golyalpha/z, can't inspect

0crat commented 5 years ago

The job #76 is now out of scope

0crat commented 5 years ago

Payment to ARC for a closed pull request, as in §28: +10 point(s) just awarded to @llorllale/z

sereshqua commented 5 years ago

@0crat quality acceptable

0crat commented 5 years ago

Quality review completed: +4 point(s) just awarded to @sereshqua/z

0crat commented 5 years ago

Order was finished, quality is "acceptable": +15 point(s) just awarded to @golyalpha/z