Enable test-integration-cli for Windows platform #9284

icecrime · 2014-11-22T02:32:38Z

This is more steps toward functional test-integration-cli for Windows client-side. How to run under Cygwin:

export CYGWIN=winsymlinks:lnk                          # Use simple .lnk for symlinks
export DOCKER_CLIENTONLY=1
export DOCKER_TEST_HOST=tcp://1.2.3.4:5678
project/make.sh binary test-integration-cli

Known issues:

Many tests fail, most likely a majority because of the test implementation itself
Coverage is not measured: I'm guessing this is another cygpath issues leading to coverfiles not being produced
This doesn't work with git-provided bash.exe: perhaps a bash expert can bring his expertise here

I believe this is worth merging as it provides a reasonable base for people to start fixing individual failing tests.

Update: how to set up a test environment on Windows:

You'll need bash.exe, and unfortunately the one provided with git installer won't work. I chose to use Cygwin out of habit, but there's probably other (and perhaps better) choices out there.
Because Windows doesn't support Docker daemon yet, you'll have to run a daemon somewhere to connect to. Remember to launch with the daemon with the appropriate -H directive allowing your Windows box to access it remotely.
Launch Cygwin (the shell defaults to bash), cd to Docker source directory, and run:
- export DOCKER_CLIENTONLY=1: indicates to the build scripts that you're running on a "client-only" platform
- export DOCKER_TEST_HOST=tcp://<docker_host>:<docker_port>: gives the build scripts the address of the remote Docker host
- project/make.sh binary test-integration-cli

Troubleshooting: the binary target should produce both a docker-$VERSION.exe binary inside ./bundles/$VERSION/binary/ and a docker.exe.lnk next to it. I encountered issues where the docker command was not recognized even though the link was in my path: this was due to Cygwin producing other types of links, and was fixed using export CYGWIN=winsymlinks:lnk.

unclejack · 2014-11-25T21:19:03Z

@icecrime Is there some documentation on how to set up an environment for testing on Windows? I'm interested in helping out with this. I also need to run tests on Windows.

tianon · 2014-11-25T21:22:14Z

project/make/test-integration-cli

-		--pidfile "$DEST/docker.pid" \
-			&> "$DEST/docker.log"
-	) &
+	if [ -z $DOCKER_TEST_HOST ]; then


"$DOCKER_TEST_HOST"

tianon · 2014-11-25T21:22:42Z

Other than that one minor quoting nit, project/make/test-integration-cli change LGTM

johngossman · 2014-11-25T22:49:18Z

integration-cli/utils.go

-	target = strings.Trim(target, "\n")
-	target = strings.Trim(target, " ")
-	return target
+	return strings.Trim(target, " \r\n")


Seems like strings.TrimSpace() is better

johngossman · 2014-11-25T22:52:36Z

Overall this LGTM

icecrime · 2014-11-26T00:02:44Z

@unclejack Indeed there is no documentation. Do you think docs/sources/contributing/devenvironment.md is a good place for this considering it's WIP and not yet an officially supported platform? I can try to write a little paragraph.

Thanks for the reviews, updating with the latest comments.

unclejack · 2014-11-26T00:04:43Z

@icecrime I think some instructions provided on this PR on how to test such PRs would be enough for now.

icecrime · 2014-12-13T00:02:57Z

Rebased! PTAL

icecrime · 2014-12-18T17:38:02Z

Ping @tianon @tiborvass

tianon · 2014-12-19T06:07:39Z

LGTM

Signed-off-by: Arnaud Porterie <arnaud.porterie@docker.com>

icecrime · 2014-12-21T00:42:05Z

Rebased again, tibor in holidays: ping @LK4D4 @jfrazelle!

SvenDowideit · 2014-12-22T03:05:42Z

@icecrime yes please - throw them into the devenv.md you mentioned :)

icecrime · 2014-12-22T16:33:01Z

If possible, I'd rather have this merged ASAP because:

It's a pain to rebase :-)
Has been opened for more than a month
I would make it possible for people at MSFT to take on the work of fixing tests.

I can add documentation in a second PR (we'll definitely need some docs updating once Windows becomes a supported client platform anyway).

jessfraz · 2014-12-22T16:42:51Z

I will take a look today :)

On Monday, December 22, 2014, Arnaud Porterie notifications@github.com
wrote:

If possible, I'd rather have this merged ASAP because:

It's a pain to rebase :-)

Has been opened for more than a month

I would make it possible for people at MSFT to take on the work of
fixing tests.

I can add documentation in a second PR (we'll definitely need some docs
updating once Windows becomes a supported client platform anyway).

—
Reply to this email directly or view it on GitHub
#9284 (comment).

jessfraz · 2014-12-22T18:59:11Z

LGTM

Enable test-integration-cli for Windows platform

johngossman · 2014-12-23T22:19:22Z

Thanks!

tianon · 2014-12-24T05:51:52Z

project/make/.integration-daemon-start

-		&> "$DEST/docker.log"
-) &
+if [ -z "$DOCKER_TEST_HOST" ]; then
+    ( set -x; exec \


You're so ruining Christmas.

Give me a minute to fix that!

This is merged already -- I'm working up a PR right now, no worries. 😉

Just wanted to bust some stones, especially @jfrazelle's.

I'll do it, no worries it's my fault (didn't pay attention when I moved the lines from integration-cli to the newly introduced files).

set list listchars=tab:»·,nbsp:_,extends:¬

in my .vimrc is basically my favorite line in the entire file ❤️

Waaaaahhhhh we need vim headers!

On Tuesday, December 23, 2014, Tianon Gravi notifications@github.com
wrote:

In project/make/.integration-daemon-start
#9284 (diff):

@@ -15,10 +15,14 @@ exec 41>&1 42>&2
DOCKER_GRAPHDRIVER=${DOCKER_GRAPHDRIVER:-vfs}
DOCKER_EXECDRIVER=${DOCKER_EXECDRIVER:-native}

-( set -x; exec \

docker --daemon --debug \

--storage-driver "$DOCKER_GRAPHDRIVER" \

--exec-driver "$DOCKER_EXECDRIVER" \

--pidfile "$DEST/docker.pid" \

&> "$DEST/docker.log"
-) &
+if [ -z "$DOCKER_TEST_HOST" ]; then

( set -x; exec \

set list listchars=tab:»·,nbsp:_,extends:¬

in my .vimrc is basically my favorite line in the entire file [image:
❤️]

—
Reply to this email directly or view it on GitHub
https://github.com/docker/docker/pull/9284/files#r22246707.

Just fix your editor config and kwitchyerbellyakin!

Bah humbug

On Tuesday, December 23, 2014, Tianon Gravi notifications@github.com
wrote:

In project/make/.integration-daemon-start
#9284 (diff):

@@ -15,10 +15,14 @@ exec 41>&1 42>&2
DOCKER_GRAPHDRIVER=${DOCKER_GRAPHDRIVER:-vfs}
DOCKER_EXECDRIVER=${DOCKER_EXECDRIVER:-native}

-( set -x; exec \

docker --daemon --debug \

--storage-driver "$DOCKER_GRAPHDRIVER" \

--exec-driver "$DOCKER_EXECDRIVER" \

--pidfile "$DEST/docker.pid" \

&> "$DEST/docker.log"
-) &
+if [ -z "$DOCKER_TEST_HOST" ]; then

( set -x; exec \

Just fix your editor config and kwitchyerbellyakin!

—
Reply to this email directly or view it on GitHub
https://github.com/docker/docker/pull/9284/files#r22246999.

tianon reviewed Nov 25, 2014
View reviewed changes

johngossman reviewed Nov 25, 2014
View reviewed changes

icecrime closed this Nov 26, 2014

icecrime reopened this Nov 26, 2014

icecrime force-pushed the client_only_integration_tests branch from 52a60e5 to 43151b0 Compare November 26, 2014 00:14

icecrime force-pushed the client_only_integration_tests branch from 43151b0 to 4e84be3 Compare December 13, 2014 00:02

icecrime force-pushed the client_only_integration_tests branch from 4e84be3 to e7db5ae Compare December 21, 2014 00:39

Enable test-integration-cli for Windows platform

f3ed422

Signed-off-by: Arnaud Porterie <arnaud.porterie@docker.com>

icecrime force-pushed the client_only_integration_tests branch from e7db5ae to f3ed422 Compare December 21, 2014 00:40

jessfraz pushed a commit that referenced this pull request Dec 22, 2014

Merge pull request #9284 from icecrime/client_only_integration_tests

4d0fcb6

Enable test-integration-cli for Windows platform

jessfraz merged commit 4d0fcb6 into moby:master Dec 22, 2014

tianon reviewed Dec 24, 2014
View reviewed changes

SvenDowideit mentioned this pull request Jan 27, 2015

Post 1.4.1 docs update 2 #9739

Merged

icecrime deleted the client_only_integration_tests branch July 15, 2015 17:49

thaJeztah mentioned this pull request Apr 19, 2016

Windows CI: Eliminate use of DOCKER_TEST_HOST #22134

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable test-integration-cli for Windows platform #9284

Enable test-integration-cli for Windows platform #9284

icecrime commented Nov 22, 2014

unclejack commented Nov 25, 2014

tianon Nov 25, 2014

tianon commented Nov 25, 2014

johngossman Nov 25, 2014

johngossman commented Nov 25, 2014

icecrime commented Nov 26, 2014

unclejack commented Nov 26, 2014

icecrime commented Dec 13, 2014

icecrime commented Dec 18, 2014

tianon commented Dec 19, 2014

icecrime commented Dec 21, 2014

SvenDowideit commented Dec 22, 2014

icecrime commented Dec 22, 2014

jessfraz commented Dec 22, 2014

jessfraz commented Dec 22, 2014

johngossman commented Dec 23, 2014

tianon Dec 24, 2014

icecrime Dec 24, 2014

tianon Dec 24, 2014

tianon Dec 24, 2014

icecrime Dec 24, 2014

tianon Dec 24, 2014

jessfraz Dec 24, 2014

tianon Dec 24, 2014

jessfraz Dec 24, 2014

Enable test-integration-cli for Windows platform #9284

Enable test-integration-cli for Windows platform #9284

Conversation

icecrime commented Nov 22, 2014

unclejack commented Nov 25, 2014

Choose a reason for hiding this comment

tianon commented Nov 25, 2014

Choose a reason for hiding this comment

johngossman commented Nov 25, 2014

icecrime commented Nov 26, 2014

unclejack commented Nov 26, 2014

icecrime commented Dec 13, 2014

icecrime commented Dec 18, 2014

tianon commented Dec 19, 2014

icecrime commented Dec 21, 2014

SvenDowideit commented Dec 22, 2014

icecrime commented Dec 22, 2014

jessfraz commented Dec 22, 2014

jessfraz commented Dec 22, 2014

johngossman commented Dec 23, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment