-
Notifications
You must be signed in to change notification settings - Fork 90
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
Run Windows CI on every PR #167
Comments
@samwright I've got AppVeyor approved so we should be unblocked to actually put Windows CI on all PRs for docker compose rule :) |
That's excellent news! You'll find the build will fail (lots of test failures) until #169 and #171 are merged. You'll also need to set the DOCKER_LOCATION and DOCKER_COMPOSE_LOCATION env variables, or merge #170 (which scans $PATH instead). It might be easier to merge those before enabling the windows build, but that's your choice. |
I've got just one more test to fix: HostNetworkedPortsIntegrationTest. This tests that having docker use the 'host' network type means the container uses the host's network, so the host can see connect to all container ports on localhost and vice-versa (afaik). I get the following:
Since docker on windows/mac is actually running inside a linux VM, the "host" is not 127.0.0.1 but the linux VM (running on 10.0.75.1), and its firewall/docker seems to prevent connecting to these ports (i.e. curl 10.0.75.1:5432 fails). Here is a discussion showing that this is intended behaviour: https://forums.docker.com/t/should-docker-run-net-host-work/14215. @iamdanfox You mentioned you're on Mac, so if I'm correct you should be having the same problem running HostNetworkedPortsIntegrationTest. Is this correct? If so I guess the best course would be to use JUnit's Assume to skip the test when on Windows/Mac. |
@samwright you are correct this doesn't work on Mac - I believe when the test was originally written it was before Docker for Mac existed and while the behaviour with docker-machine wasn't great it was at least functional. Using an assume here seems like the best thing to do. |
Just an update, docker-compose-rule now builds on Windows and passes all tests :-D |
@samwright excellent. Unfortunately appveyor only supports Windows containers so I'm adding a quick smoke test that uses a Windows base image to ensure we don't regress in the future. |
@hpryce @iamdanfox - looks like this is resolved - just want to double check and make sure that's the case. We good to close this out? |
We're issuing commands which need different escaping on Mac / windows. Let's get some AppVeyor CI going.
#166
The text was updated successfully, but these errors were encountered: