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

integ-cli: remove bash dependency from TestSaveDirectoryPermissions #10997

Merged
merged 1 commit into from Feb 25, 2015
Merged

integ-cli: remove bash dependency from TestSaveDirectoryPermissions #10997

merged 1 commit into from Feb 25, 2015

Conversation

@ahmetb
Copy link
Contributor

@ahmetb ahmetb commented Feb 24, 2015

Use the new runCommandPipelineWithOutput helper to
remove bash dependency required for piping in
TestSaveDirectoryPermissions.

It wasn't working first time I tried it (test was getting stuck
indefinitely) magically works now.

Signed-off-by: Ahmet Alp Balkan ahmetalpbalkan@gmail.com
cc: @jfrazelle @tianon @duglin @LK4D4

Use the new `runCommandPipelineWithOutput` helper to
remove bash dependency required for piping in
`TestSaveDirectoryPermissions`.

Signed-off-by: Ahmet Alp Balkan <ahmetalpbalkan@gmail.com>
@tianon
Copy link
Member

@tianon tianon commented Feb 25, 2015

Seems legit 👍

Is there a reason why we shell out and then verify a directory on-disk instead of using archive/tar to read the tar stream directly? (as a curiosity, not a suggestion for this PR)

@tiborvass
Copy link
Collaborator

@tiborvass tiborvass commented Feb 25, 2015

@tianon coz thats what people use when they untar? :P idk, i'm fine either way.

LGTM

@jessfraz
Copy link
Contributor

@jessfraz jessfraz commented Feb 25, 2015

LGTM

jessfraz pushed a commit that referenced this issue Feb 25, 2015
…ryPermissions-fix

integ-cli: remove bash dependency from TestSaveDirectoryPermissions
@jessfraz jessfraz merged commit 8974f61 into moby:master Feb 25, 2015
1 check passed
@ahmetb ahmetb deleted the win-cli/TestSaveDirectoryPermissions-fix branch Feb 25, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants