Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#111 FIX: VerboseIterable and VerboseIterator are not tested #218

Merged
merged 28 commits into from
May 7, 2015
Merged

#111 FIX: VerboseIterable and VerboseIterator are not tested #218

merged 28 commits into from
May 7, 2015

Conversation

marcuss
Copy link

@marcuss marcuss commented Apr 29, 2015

@yegor256 This pull request fixes bug #111 that identified the missing unit tests in VerboseIterable and VerboseIterator, for fixing this, the VerboseIterableTest and VerboseIteratorTest classes were created.

@marcuss
Copy link
Author

marcuss commented Apr 29, 2015

@yegor256 this is my first pull request, I see the CI kicking in, but there is apparently a non deterministic test on
FtBasicTest.gracefullyHandlesStuckBack

I can't reproduce it locally, but since I din't change code, just added unit test is not normal that my build has errors

:144 HTTP response body content is not valid:
200 OK [http://localhost:1050/]
Content-Type: text/plain

first: , second:
Expected: a string containing "second: dd"
but: was "first: , second: "

@davvd
Copy link

davvd commented Apr 29, 2015

@M4Solutions Thanks for your pull request, let me find someone who can review it

@davvd
Copy link

davvd commented Apr 29, 2015

@pinaf it's yours, please review

"Accept-Charset: utf-8",
"Authorization: Basic QWxhZGRpbjpvcGVuIHNlc2FtZQ==",
"Cache-Control: no-cache", "From: user@example.com");

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@M4Solutions Add a javadoc comment here with the form VerboseIterable can .... I suggest:

/**
 * VerboseIterable can return correct size.
 */

@pinaf
Copy link

pinaf commented Apr 29, 2015

@M4Solutions 20 comments and 1 question above.

@yegor256
Copy link
Owner

yegor256 commented May 4, 2015

@rultor try to merge

@rultor
Copy link
Collaborator

rultor commented May 4, 2015

@rultor try to merge

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

@rultor
Copy link
Collaborator

rultor commented May 4, 2015

@rultor try to merge

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

Running org.takes.facets.auth.social.PsTwitterTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 1, Time elapsed: 0 sec - in org.takes.facets.auth.social.PsTwitterTest

Results :

Tests in error: 
  RqMultipartTest.consumesHttpRequest:303 » IO Failed POST request to http://loc...

Tests run: 168, Failures: 0, Errors: 1, Skipped: 4

[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 53.675 s
[INFO] Finished at: 2015-05-04T04:41:57+00:00
[INFO] Final Memory: 27M/388M
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:2.17:test (default-test) on project takes: There are test failures.
[ERROR] 
[ERROR] Please refer to /home/r/repo/target/surefire-reports for the individual test results.
[ERROR] -> [Help 1]
org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:2.17:test (default-test) on project takes: There are test failures.

Please refer to /home/r/repo/target/surefire-reports for the individual test results.
    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:120)
    at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:347)
    at org.apache.maven.DefaultMaven.execute(DefaultMaven.java:154)
    at org.apache.maven.cli.MavenCli.execute(MavenCli.java:584)
    at org.apache.maven.cli.MavenCli.doMain(MavenCli.java:213)
    at org.apache.maven.cli.MavenCli.main(MavenCli.java:157)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:606)
    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: There are test failures.

Please refer to /home/r/repo/target/surefire-reports for the individual test results.
    at org.apache.maven.plugin.surefire.SurefireHelper.reportExecution(SurefireHelper.java:82)
    at org.apache.maven.plugin.surefire.SurefirePlugin.handleSummary(SurefirePlugin.java:195)
    at org.apache.maven.plugin.surefire.AbstractSurefireMojo.executeAfterPreconditionsChecked(AbstractSurefireMojo.java:861)
    at org.apache.maven.plugin.surefire.AbstractSurefireMojo.execute(AbstractSurefireMojo.java:729)
    at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo(DefaultBuildPluginManager.java:132)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:208)
    ... 19 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

@marcuss
Copy link
Author

marcuss commented May 7, 2015

@pinaf Should I close this pull request now that the problem in master has been skipped?

@pinaf
Copy link

pinaf commented May 7, 2015

@rultor merge
@marcuss we should try to merge again

@rultor
Copy link
Collaborator

rultor commented May 7, 2015

@rultor merge
@marcuss we should try to merge again

@pinaf Thanks for your request. @yegor256 Please confirm this.

@pinaf
Copy link

pinaf commented May 7, 2015

@marcuss @yegor256 both of you have unaddressed comments. please go through your comments and make sure all of them address somebody.

@marcuss
Copy link
Author

marcuss commented May 7, 2015

@pinaf I just corrected my unaddressed comments.

@yegor256
Copy link
Owner

yegor256 commented May 7, 2015

@marcuss see above

@yegor256
Copy link
Owner

yegor256 commented May 7, 2015

@rultor merge

@rultor
Copy link
Collaborator

rultor commented May 7, 2015

@rultor merge

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

@rultor rultor merged commit cadb7c4 into yegor256:master May 7, 2015
@rultor
Copy link
Collaborator

rultor commented May 7, 2015

@rultor merge

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

@pinaf
Copy link

pinaf commented May 7, 2015

@marcuss thanks. also make sure that the first post explains what issue this is for and what your solution was. here is some important info on QA http://www.teamed.io/qa.html

@davvd
Copy link

davvd commented May 8, 2015

@elenavolokhova review this ticket please, for compliance with our quality rules

@elenavolokhova
Copy link

@pinaf Thank you for your job on quality of this endless review :) But one issue is still not fixed: @yegor256 comments still don't have addressee. Please, try to be more persistent next time.

@davvd
Copy link

davvd commented May 9, 2015

@rultor deploy

@rultor
Copy link
Collaborator

rultor commented May 9, 2015

@rultor deploy

@davvd OK, I'll try to deploy now. You can check the progress here

@rultor
Copy link
Collaborator

rultor commented May 9, 2015

@rultor deploy

@davvd Done! FYI, the full log is here (took me 10min)

@pinaf
Copy link

pinaf commented May 9, 2015

@elenavolokhova alright.
@yegor256 please address your unaddressed comments :)

@yegor256
Copy link
Owner

yegor256 commented May 9, 2015

@elenavolokhova yes, I got it, will be more attentive in the future, thanks!

@elenavolokhova
Copy link

@davvd Quality is acceptable here.

@davvd
Copy link

davvd commented May 11, 2015

@davvd Quality is acceptable here.

@elenavolokhova thanks for the review, we'll try better next time

@davvd
Copy link

davvd commented May 11, 2015

@pinaf just added 10 mins to @elenavolokhova (for QA), payment ID is 57161463... Much obliged! I have added 26 mins to your account in payment "AP-5SD91831XV3762709", 201 hours and 34 mins spent... extra minutes for review comments (c=11)... added +26 to your rating, now it is equal to +7835

@0pdd
Copy link
Collaborator

0pdd commented Oct 11, 2018

@marcuss all 2 puzzles are solved here: #780, #806.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants