Skip to content

#2141 fix travis - #1255

Merged
DonJayamanne merged 39 commits into
masterfrom
2141FixTravis
Oct 3, 2017
Merged

#2141 fix travis#1255
DonJayamanne merged 39 commits into
masterfrom
2141FixTravis

Conversation

@DonJayamanne

Copy link
Copy Markdown
Owner

The fix was scoped to .travis.yml
However I couldn't help but clean some of the code (not much, but a little).
Also fixed the unittest tests.

@DonJayamanne DonJayamanne added this to the October 2017 milestone Sep 28, 2017
@brettcannon

Copy link
Copy Markdown
Contributor

Looks like tests are still failing. 😢

@DonJayamanne

Copy link
Copy Markdown
Owner Author

Yes, the plan was to get the tests running and not fixing them.
I'll fix them, as well.

@DonJayamanne

Copy link
Copy Markdown
Owner Author

@brettcannon
Yay fixed. Couldn't figure out why the behaviour for unittests module was different on travis (locally they work as expexted on P2.7.1 on Windows and Linux).
Changed the tests to remove Failures and exceptions being thrown from the unit tests.

Comment thread .travis.yml
@@ -4,7 +4,16 @@ dist: trusty

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add cache: pip to cache pip-installed dependencies.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

Comment thread .travis.yml
pyenv install $PYTHON
pyenv virtualenv $PYTHON MYVERSION
source ~/.pyenv/versions/MYVERSION/bin/activate
pyenv global $PYTHON

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is pyenv for? If you need to run tests under different Python versions you could use tox and then list different Python versions in the text matrix.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the only way I knew I could get multiple versions of Python running on Travis for OSX. Will check tox.

Comment thread .travis.yml Outdated
# we have this here so we can see where python is installed and hardcode in our tests
# else when running npm test, the python version picked is completely different :(
# python-dev is to ensure jupyter installs properly
- sudo apt-get install python-dev

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you not specify python-dev under packages so you can drop the sudo requirement? I'm actually surprised it isn't included by default.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some reason this is required, else installing jupyter falls over.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hrm, that's unfortunate/weird. Another reason to eventually rip that code out! 😉

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes agreed.
However I would like to remove Jupyter tests and all of the related code/settings in a separate PR. I'd like to track that separately. what do you think?
I.e. leave this for now, and it will get removed in another PR.

Comment thread .travis.yml Outdated
- sudo apt-get install python-dev
- python --version
- python -c 'import sys;print(sys.executable)'
- pip install -r requirements.txt

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can also use python -m pip to guarantee what version of Python you're using.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how do i do that?

Comment thread .travis.yml Outdated
# else when running npm test, the python version picked is completely different :(
# python-dev is to ensure jupyter installs properly
- sudo apt-get install python-dev
- python --version

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be in the Travis output based on what version of Python you specified.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

Comment thread .travis.yml Outdated
python -c 'import sys;print(sys.executable)'
fi
install:
# we have this here so we can see where python is installed and hardcode in our tests

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hardcode how? Can you just pass it in through an environment variable or command-line option? E.g.

PYTHON_EXE=`which python`

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was for OSX tests. I need this to be the global setting so the unit tests can pick the standard python path. I could introduce this env variable as well.

@DonJayamanne

Copy link
Copy Markdown
Owner Author

@brettcannon done

@brettcannon brettcannon left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor change, but otherwise this LGTM!

Comment thread .travis.yml Outdated
# else when running npm test, the python version picked is completely different :(
- python --version
- python -c 'import sys;print(sys.executable)'
- pip install -r requirements.txt

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably want pip install --upgrade -r requirements.txt for when unpinned dependencies change.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@DonJayamanne
DonJayamanne merged commit ba2ccab into master Oct 3, 2017
@DonJayamanne
DonJayamanne deleted the 2141FixTravis branch October 11, 2017 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants