Skip to content

Conversation

@dnephin
Copy link

@dnephin dnephin commented Aug 25, 2015

@mnowster this should fix it

@mnowster
Copy link

LGTM! I don't know enough about the history between script/tests and script/test-versions to know if this is going to cause any problems, I don't think so but would like @aanand to check to be sure :)

@aanand
Copy link

aanand commented Aug 25, 2015

I think the change to script/test should be fine. Only problem is we run tox outside of a container, which requires it (and, by extension, a Python installation) on the host machine.

Could we wrap it in a docker run? We'd need to mount $(pwd)/.git into the container as well so that the hooks can run, as I understand it.

@dnephin
Copy link
Author

dnephin commented Aug 25, 2015

Yup, that should work, I'll make that change

@dnephin dnephin mentioned this pull request Aug 25, 2015
@dnephin
Copy link
Author

dnephin commented Aug 25, 2015

script/test-versions now runs tox in the container, so it doesn't need to be installed on the host.

@aanand
Copy link

aanand commented Aug 26, 2015

Looks like something's up with one of the git-related hooks:

+ docker run --rm -t --privileged --volume=/var/jenkins_home/workspace/Compose-PRs/.git:/code/.git --name compose-prs1130 --volume=/var/run/docker.sock:/var/run/docker.sock -e DOCKER_VERSIONS=all -e TAG=compose:d4711f0 --entrypoint=script/ci compose:d4711f0 --verbose
Running lint checks
pre-commit create: /code/.tox/pre-commit
pre-commit installdeps: pre-commit
pre-commit develop-inst: /code
pre-commit installed: aspy.yaml==0.2.1,cached-property==1.2.0,## !! Could not determine repository location,docker-compose==1.5.0.dev0,docker-py==1.3.1,dockerpty==0.3.4,docopt==0.6.2,functools32==3.2.3.post2,jsonschema==2.5.1,nodeenv==0.13.3,ordereddict==1.1,pre-commit==0.5.4,PyYAML==3.11,requests==2.6.2,simplejson==3.8.0,six==1.9.0,texttable==0.8.3,virtualenv==13.1.2,websocket-client==0.32.0,wheel==0.24.0
pre-commit runtests: PYTHONHASHSEED='2635628559'
pre-commit runtests: commands[0] | pre-commit install
pre-commit installed at /code/.git/hooks/pre-commit
tput: No value for $TERM and no -T specified
pre-commit runtests: commands[1] | pre-commit run --all-files
tput: No value for $TERM and no -T specified
An unexpected error has occurred: CalledProcessError: Command: ['git', 'ls-files', '--unmerged']
Return code: 128
Expected return code: 0
Output: (none)
Errors: 
    fatal: Not a git repository (or any of the parent directories): .git


Check the log at ~/.pre-commit/pre-commit.log
ERROR: InvocationError: '/code/.tox/pre-commit/bin/pre-commit run --all-files'
___________________________________ summary ____________________________________
ERROR:   pre-commit: commands failed

@dnephin
Copy link
Author

dnephin commented Aug 26, 2015

Finally a green build! How does this look?

I've handled the volume problem (difference between scripts/ci and scripts/test) by making it an environment variable

@aanand
Copy link

aanand commented Aug 27, 2015

I'm getting a really strange problem when running script/test locally on this branch: on Python 2.7, pre-commit seems to install docker-py==0.5.3.

++ docker run --rm --privileged --volume=/var/lib/docker --volume=/Users/aanand/work/docker/compose/coverage-html:/code/coverage-html -e DOCKER_VERSION=default -e DOCKER_DAEMON_ARGS --entrypoint=script/dind docker-compose:6301113 script/wrapdocker tox -e py27,py34 --
Starting Docker with: docker -d
Waiting for Docker to start...
> tox -e py27,py34 --
py27 create: /code/.tox/py27
py27 installdeps: -rrequirements.txt, -rrequirements-dev.txt
py27 develop-inst: /code
py27 installed: coverage==3.7.1,docker-compose==1.5.0.dev0,docker-py==0.5.3,dockerpty==0.3.4,docopt==0.6.1,fig==1.0.0,flake8==2.3.0,funcsigs==0.4,functools32==3.2.3.post2,jsonschema==2.5.1,mccabe==0.3.1,mock==1.3.0,nose==1.3.4,pbr==1.6.0,pep8==1.6.1,pyflakes==0.9.2,PyInstaller===2.1.1dev-12e4047,PyYAML==3.10,requests==2.6.1,six==1.7.3,texttable==0.8.2,websocket-client==0.11.0,wheel==0.24.0

This of course causes many test failures. It doesn't happen on Python 3.4:

py34 create: /code/.tox/py34
py34 installdeps: -rrequirements.txt, flake8, nose
py34 develop-inst: /code
py34 installed: docker-compose==1.5.0.dev0,docker-py==1.3.1,dockerpty==0.3.4,docopt==0.6.1,fig==1.0.0,flake8==2.4.1,jsonschema==2.5.1,mccabe==0.3.1,nose==1.3.7,pep8==1.5.7,pyflakes==0.8.1,PyYAML==3.10,requests==2.6.1,six==1.7.3,texttable==0.8.2,websocket-client==0.32.0,wheel==0.24.0

@aanand
Copy link

aanand commented Aug 27, 2015

OK, I think it was caused by either a stale docker-compose.spec or docker-compose.egg-info. Adding that to .dockerignore fixed it - see #1935.

@dnephin
Copy link
Author

dnephin commented Aug 27, 2015

Rebased now that #1935 is merged. Let's see if I can get a green run

@dnephin
Copy link
Author

dnephin commented Aug 31, 2015

The only test failure here is the one fixed by #1933

Signed-off-by: Daniel Nephin <dnephin@gmail.com>
… of launching another container.

Signed-off-by: Daniel Nephin <dnephin@gmail.com>
@dnephin
Copy link
Author

dnephin commented Sep 1, 2015

Opened #1955 for this other test that keeps failing

@dnephin
Copy link
Author

dnephin commented Sep 2, 2015

Green build! ready for more eyes

@aanand
Copy link

aanand commented Sep 2, 2015

LGTM

aanand added a commit that referenced this pull request Sep 2, 2015
Fix scripts/test and add to note about pre-commit hook to contributing
@aanand aanand merged commit 027414c into docker:master Sep 2, 2015
@dnephin dnephin deleted the fix_scripts_test branch September 2, 2015 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants