Skip to content

Conversation

amihaiemil
Copy link
Owner

PR for #256. Apparently, it is mandatory to specify either stdout or stderr when reading the logs. Left some puzzles.

@0crat 0crat added the scope label Jan 3, 2019
@0crat
Copy link
Collaborator

0crat commented Jan 3, 2019

Job #257 is now in scope, role is REV

@0crat
Copy link
Collaborator

0crat commented Jan 3, 2019

This pull request #257 is assigned to @paulodamaso/z, here is why; the budget is 15 minutes, see §4; please, read §27 and when you decide to accept the changes, inform @amihaiemil/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 a monetary reward for this job

@coveralls
Copy link

Pull Request Test Coverage Report for Build 455

  • 7 of 7 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.09%) to 89.348%

Totals Coverage Status
Change from base Build 453: 0.09%
Covered Lines: 671
Relevant Lines: 751

💛 - Coveralls

@coveralls
Copy link

coveralls commented Jan 3, 2019

Pull Request Test Coverage Report for Build 459

  • 7 of 7 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.09%) to 89.348%

Totals Coverage Status
Change from base Build 453: 0.09%
Covered Lines: 671
Relevant Lines: 751

💛 - Coveralls

Copy link
Contributor

@paulodamaso paulodamaso left a comment

Choose a reason for hiding this comment

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

@amihaiemil Have some comments, please take a look

/**
* Integration tests for {@link RtLogs}.
* @author Mihai Andronache (amihaiemil@gmail.com)
* @version $Id#
Copy link
Contributor

Choose a reason for hiding this comment

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

@amihaiemil $Id$ here

final String logs = container.logs().fetch();
MatcherAssert.assertThat(
logs,
Matchers.not(Matchers.isEmptyOrNullString())
Copy link
Contributor

Choose a reason for hiding this comment

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

@amihaiemil I'd prefer to use non-static matchers, makes the code more elegant

);
MatcherAssert.assertThat(
logs.trim(),
Matchers.startsWith("Hello from Docker!")
Copy link
Contributor

Choose a reason for hiding this comment

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

@amihaiemil Same as above

final String logs = IOUtils.toString(container.logs().follow());
MatcherAssert.assertThat(
logs,
Matchers.not(Matchers.isEmptyOrNullString())
Copy link
Contributor

Choose a reason for hiding this comment

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

@amihaiemil Here too, no static matchers

);
MatcherAssert.assertThat(
logs.trim(),
Matchers.startsWith("Hello from Docker!")
Copy link
Contributor

Choose a reason for hiding this comment

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

@amihaiemil Here again

Copy link
Contributor

@paulodamaso paulodamaso left a comment

Choose a reason for hiding this comment

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

@amihaiemil Very good, thanks

@paulodamaso
Copy link
Contributor

@rultor merge

@rultor
Copy link
Collaborator

rultor commented Jan 3, 2019

@rultor merge

@paulodamaso Thanks for your request. @amihaiemil Please confirm this.

@amihaiemil
Copy link
Owner Author

@rultor merge it

@rultor
Copy link
Collaborator

rultor commented Jan 3, 2019

@rultor merge it

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

@rultor rultor merged commit 1dc8658 into master Jan 3, 2019
@amihaiemil amihaiemil deleted the 256 branch January 3, 2019 20:48
@rultor
Copy link
Collaborator

rultor commented Jan 3, 2019

@rultor merge it

@amihaiemil Done! FYI, the full log is here (took me 2min)

@0crat
Copy link
Collaborator

0crat commented Jan 3, 2019

Job was finished in 0 hours, bonus for fast delivery is possible (see §36)

@0crat 0crat removed the scope label Jan 3, 2019
@0crat
Copy link
Collaborator

0crat commented Jan 3, 2019

The job #257 is now out of scope

@0crat
Copy link
Collaborator

0crat commented Jan 3, 2019

Order was finished: +20 point(s) just awarded to @paulodamaso/z

@0crat
Copy link
Collaborator

0crat commented Jan 3, 2019

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

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

Successfully merging this pull request may close these issues.

5 participants