Skip to content

Conversation

jackpan123
Copy link
Contributor

fix #348

@coveralls
Copy link

coveralls commented Aug 5, 2020

Pull Request Test Coverage Report for Build 701

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

Totals Coverage Status
Change from base Build 696: 0.05%
Covered Lines: 1033
Relevant Lines: 1141

💛 - Coveralls

Copy link
Owner

@amihaiemil amihaiemil left a comment

Choose a reason for hiding this comment

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

@jackpan123 Please see comments.

@Override
public Container get(final String containerId) {
return new RtContainer(
Json.createObjectBuilder().build(),
Copy link
Owner

Choose a reason for hiding this comment

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

@jackpan123 can you add the id to this JSON?

Json.createObjectBuilder()
        .add("Id", containerId)
        .build()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I will.

URI.create("http://localhost/test"),
Mockito.mock(Docker.class)
).get("df2419f4").stop();
}
Copy link
Owner

Choose a reason for hiding this comment

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

@jackpan123 These 2 tests don't make sense. Why are you calling start and stop?

Just add a simple test only for the get(...) method and assert that a Container is being returned (not null) and that its JSON contains the ID.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What get(...) method wants to express is that users cant directly obtain a certain container instead of iteratively searching. When the user finds it, who can directly manipulate the internal method of the Container .

@jackpan123 jackpan123 requested a review from amihaiemil August 6, 2020 04:58
@amihaiemil
Copy link
Owner

@rultor merge it

@rultor
Copy link
Collaborator

rultor commented Aug 7, 2020

@rultor merge it

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

@rultor rultor merged commit 996d904 into amihaiemil:master Aug 7, 2020
@rultor
Copy link
Collaborator

rultor commented Aug 7, 2020

@rultor merge it

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

@0crat
Copy link
Collaborator

0crat commented Aug 7, 2020

Job gh:amihaiemil/docker-java-api#350 is not assigned, can't get performer

@0crat 0crat removed the 0crat/scope label Aug 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add get method in Containers Interface
5 participants