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

integration-cli: Fix path problems in TestBuildRenamedDockerfile #10873

Merged
merged 1 commit into from
Feb 20, 2015
Merged

integration-cli: Fix path problems in TestBuildRenamedDockerfile #10873

merged 1 commit into from
Feb 20, 2015

Conversation

ahmetb
Copy link
Contributor

@ahmetb ahmetb commented Feb 18, 2015

TestBuildRenamedDockerfile tests hard-code unix-style
path building. Made use of path/filepath to make these
tests work on Windows as well.

Signed-off-by: Ahmet Alp Balkan ahmetalpbalkan@gmail.com
cc: @duglin @unclejack @jfrazelle @icecrime @tiborvass

@ahmetb
Copy link
Contributor Author

ahmetb commented Feb 18, 2015

https://jenkins.dockerproject.com/job/Windows-PRs/55/console
=== RUN TestBuildRenamedDockerfile
[PASSED]: build - rename dockerfile
--- PASS: TestBuildRenamedDockerfile (2.75s) 👍

if err != nil {
t.Fatal(err)
}
out, _, err = dockerCmdInDir(t, ctx.Dir, "build", fmt.Sprintf("--file=%s", dirWithNoDockerfile), "-t", "test5", ".")
Copy link
Contributor

Choose a reason for hiding this comment

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

This one is supposed to point to an existing file so I'm not sure this is the same testcase.
We may need to create a temp dir+file and then point to that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@duglin ah, makes sense. For some reason I guess I thought /etc/passwd as a dir. :-/ I'll fix this.

@duglin
Copy link
Contributor

duglin commented Feb 18, 2015

You may want to wait for #10555 to get merged first.

@ahmetb
Copy link
Contributor Author

ahmetb commented Feb 18, 2015

@duglin makes sense. please get it merged :P

@duglin
Copy link
Contributor

duglin commented Feb 18, 2015

@ahmetalpbalkan I'm twisting @tiborvass 's arm as hard as I can :-)

@jessfraz
Copy link
Contributor

ok that was merged you wanna update?

`TestBuildRenamedDockerfile` tests hard-code unix-style
path building. Made use of `path/filepath` to make these
tests work on Windows as well.

Signed-off-by: Ahmet Alp Balkan <ahmetalpbalkan@gmail.com>
@ahmetb
Copy link
Contributor Author

ahmetb commented Feb 19, 2015

@jfrazelle rebased on top of master and pushed.

@LK4D4
Copy link
Contributor

LK4D4 commented Feb 19, 2015

Seems like all failed :)

@jessfraz
Copy link
Contributor

well its failing bc we need #10904 ;)

@jessfraz
Copy link
Contributor

LGTM

@jessfraz
Copy link
Contributor

ping @duglin

@ahmetb
Copy link
Contributor Author

ahmetb commented Feb 20, 2015

https://jenkins.dockerproject.com/job/Windows-PRs/69/console
=== RUN TestBuildRenamedDockerfile
[PASSED]: build - rename dockerfile
--- PASS: TestBuildRenamedDockerfile (3.09s) 👍

@duglin
Copy link
Contributor

duglin commented Feb 20, 2015

Yup - LGTM

jessfraz pushed a commit that referenced this pull request Feb 20, 2015
…dDockerfile-fix

integration-cli: Fix path problems in TestBuildRenamedDockerfile
@jessfraz jessfraz merged commit e22487d into moby:master Feb 20, 2015
@ahmetb ahmetb deleted the win-cli/TestBuildRenamedDockerfile-fix branch February 20, 2015 00:42
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.

6 participants