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: Skip some unix-specific cli tests #10913

Merged
merged 2 commits into from Feb 20, 2015

Conversation

Projects
None yet
6 participants
@ahmetb
Contributor

ahmetb commented Feb 20, 2015

Skipping some of the tests closely tied to running in a
unix environment. Windows does not support chmod/chown
and this causes some tests to fail creating desired
behavior.

  • TestBuildWithInaccessibleFilesInContext: uses chown/chmod
  • TestBuildDockerfileOutsideContext: uses os.Symlink, not implemented on
    windows
  • TestCpUnprivilegedUser: uses chmod, and requires 'unprivilegeduser'
    created by Dockerfile (and thus requires to run inside container)
  • TestBuildChownSingleFile: uses chown

I wonder if I can write windows equivalents of those using golang
pkg/os calls for windows. Most of them are not implemented, especially
windows-specific security ACLs etc.

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

const (
// idetifies if test suite is running on a unix platform
isUnixCli = false

This comment has been minimized.

@LK4D4

LK4D4 Feb 20, 2015

Contributor

Hmm, seems like it can be just isUnix?

This comment has been minimized.

@ahmetb

ahmetb Feb 20, 2015

Contributor

Being more explicit never hurts, does it? :-)

This comment has been minimized.

@ahmetb

ahmetb Feb 20, 2015

Contributor

Also at some point I can see us running against windows daemon, so isUnix would probably get refactored to this anyway to clarify what it actually refers to.

This comment has been minimized.

@LK4D4

LK4D4 Feb 20, 2015

Contributor

Was pretty implicit for me actually :P But now I got idea.

@@ -0,0 +1,8 @@
// +build windows

This comment has been minimized.

@LK4D4

LK4D4 Feb 20, 2015

Contributor

I think you don't need tag if you use filename suffix.

This comment has been minimized.

@ahmetb

ahmetb Feb 20, 2015

Contributor

While porting docker cli to windows we decided to explicitly keep those tags explicitly even though file names do the same thing.

@@ -0,0 +1,8 @@
// +build !windows

This comment has been minimized.

@LK4D4

LK4D4 Feb 20, 2015

Contributor

I see that in standard library tags:

// +build darwin dragonfly freebsd linux nacl netbsd openbsd solaris

treated as unix, but maybe !windows is the same :) apart from android tag.

This comment has been minimized.

@ahmetb

ahmetb Feb 20, 2015

Contributor

Just following the conversation from rest of the code base. This is probably fine and if it's not it's will be a compilation error which can be easily fixed.

@LK4D4

This comment has been minimized.

Contributor

LK4D4 commented Feb 20, 2015

LGTM

@ahmetb

This comment has been minimized.

Contributor

ahmetb commented Feb 20, 2015

@LK4D4 you made me notice a typo. Can you please re-LGTM? :)

@@ -17,6 +17,10 @@ var (
func() bool { return isLocalDaemon },
"Test requires docker daemon to runs on the same machine as CLI",
}
UnixCli = TestRequirement{
func() bool { return isLocalDaemon },

This comment has been minimized.

@LK4D4

LK4D4 Feb 20, 2015

Contributor

hmmmm isLocalDaemon?

This comment has been minimized.

@ahmetb

ahmetb Feb 20, 2015

Contributor

damn. copypasta is evil.

ahmetb added some commits Feb 20, 2015

integ-cli: Skip some unix-specific cli tests
Skipping some of the tests closely tied to running in a
unix environment. Windows does not support chmod/chown
and this causes some tests to fail creating desired
behavior.

- `TestBuildWithInaccessibleFilesInContext`: uses chown/chmod
- `TestBuildDockerfileOutsideContext`: uses os.Symlink, not implemented on
  windows
- `TestCpUnprivilegedUser`: uses chmod, and requires 'unprivilegeduser'
  created by Dockerfile (and thus requires to run inside container)
- `TestBuildChownSingleFile`: uses chown

Signed-off-by: Ahmet Alp Balkan <ahmetalpbalkan@gmail.com>
integ-cli: Typo fix in test_vars_* comments.
Signed-off-by: Ahmet Alp Balkan <ahmetb@microsoft.com>
@LK4D4

This comment has been minimized.

Contributor

LK4D4 commented Feb 20, 2015

LGTM

@jessfraz

This comment has been minimized.

Contributor

jessfraz commented Feb 20, 2015

lgtm

jessfraz pushed a commit that referenced this pull request Feb 20, 2015

Jessie Frazelle
Merge pull request #10913 from ahmetalpbalkan/win-cli/UnixSpecific-skip
integ-cli: Skip some unix-specific cli tests

@jessfraz jessfraz merged commit 695bf33 into moby:master Feb 20, 2015

1 check passed

janky Jenkins build Docker-PRs 1517 has succeeded
Details

@ahmetb ahmetb deleted the ahmetb:win-cli/UnixSpecific-skip branch Feb 20, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment