Conversation
|
Looks like tests are still failing. 😢 |
|
Yes, the plan was to get the tests running and not fixing them. |
|
@brettcannon |
| @@ -4,7 +4,16 @@ dist: trusty | |||
|
|
|||
There was a problem hiding this comment.
Add cache: pip to cache pip-installed dependencies.
| pyenv install $PYTHON | ||
| pyenv virtualenv $PYTHON MYVERSION | ||
| source ~/.pyenv/versions/MYVERSION/bin/activate | ||
| pyenv global $PYTHON |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
This was the only way I knew I could get multiple versions of Python running on Travis for OSX. Will check tox.
.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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
For some reason this is required, else installing jupyter falls over.
There was a problem hiding this comment.
Hrm, that's unfortunate/weird. Another reason to eventually rip that code out! 😉
There was a problem hiding this comment.
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.
.travis.yml
Outdated
| - sudo apt-get install python-dev | ||
| - python --version | ||
| - python -c 'import sys;print(sys.executable)' | ||
| - pip install -r requirements.txt |
There was a problem hiding this comment.
You can also use python -m pip to guarantee what version of Python you're using.
.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 |
There was a problem hiding this comment.
This should be in the Travis output based on what version of Python you specified.
.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 |
There was a problem hiding this comment.
Hardcode how? Can you just pass it in through an environment variable or command-line option? E.g.
PYTHON_EXE=`which python`There was a problem hiding this comment.
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.
|
@brettcannon done |
brettcannon
left a comment
There was a problem hiding this comment.
One minor change, but otherwise this LGTM!
.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 |
There was a problem hiding this comment.
Probably want pip install --upgrade -r requirements.txt for when unpinned dependencies change.
The fix was scoped to
.travis.ymlHowever I couldn't help but clean some of the code (not much, but a little).
Also fixed the unittest tests.