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

issue 957: run as non-root user in docker #959

Merged
merged 1 commit into from
Mar 8, 2017

Conversation

gonewest818
Copy link
Contributor

@gonewest818 gonewest818 commented Mar 5, 2017

This PR resolves Issue #957 in which one of the unit tests fails because it has root privileges in docker. With this patch maven runs in docker as a non-root user named "builder" instead.

@auhlig
Copy link
Member

auhlig commented Mar 5, 2017

Hey @gonewest818,
Thanks for working on this.
But I can still see it failing https://travis-ci.org/ContainX/openstack4j/jobs/207814369#L2845

@gonewest818
Copy link
Contributor Author

gonewest818 commented Mar 5, 2017

Well, in my understanding of the test case, I think that's a "success":

 public void DownloadImage() throws IOException {
     respondWith(200);
     String imageId = "4b434528-032b-4467-946c-b5880ce15c06";
     URI uri = null;
     try {
         uri = new URI("file:////test.iso");
     }catch (URISyntaxException e) {
         e.printStackTrace();
     }
     File file = new File(uri);
     ActionResponse download = osv3().imagesV2().download(imageId, file);
     // Should fail to write to file
     assertEquals(download.getCode(), 400);
 }

The respondWith(200) mocks the GET /images/{imageId}/file that is going to occur inside the imagesV2().download(). In the Travis log you see the result of that mock

INFO: MockWebServer[9292] received request: GET /v2/images/4b434528-032b-4467-946c-b5880ce15c06/file HTTP/1.1 and responded: HTTP/1.1 200 OK

Next, URI "file:////test.iso" is used to describe the file path where the downloaded bytes will be written. However /test.iso does not exist and cannot be written. This means constructing the output stream throws an exception, which is properly caught and the exception handler prints a stack trace like you see in Travis.

The exception handler in imagesV2().download() always returns 400. So the unit test wraps up with an assertion confirming the 400 and the test therefore "succeeds."

The question is really, is that unit test coverage valid in your mind? Because this PR makes it so that test does what it intends even in the docker case.

@auhlig
Copy link
Member

auhlig commented Mar 8, 2017

Makes sense. Thanks for fixing that @gonewest818.

@auhlig auhlig merged commit 566ed7c into ContainX:master Mar 8, 2017
@auhlig auhlig added this to the 3.0.4 Release milestone Mar 8, 2017
@auhlig auhlig added the bug label Mar 8, 2017
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.

2 participants