search

graphite-project / carbon

Carbon is one of the components of Graphite, and is responsible for receiving metrics over the network and writing them down to disk using a storage backend.
http://graphite.readthedocs.org/
Apache License 2.0
1.5k stars 490 forks source link

Another test fix try #874

Closed deniszh closed 4 years ago

deniszh commented 4 years ago

Ok, by default Travis use system pip and virtualenv. I tried to upgrade it before running tests - it works fine now. Should we do that for all envs? Or keep it only for 2.7? @piotr1212 @DanCech @ploxiln ?

deniszh commented 4 years ago

Looking for Graphite-web results I think it's a good idea to test on latest pip and virtualenv. When pip deprecate 2.7 we can exclude it or pin version.

ploxiln commented 4 years ago

I'd lean towards just upgrading pip/virtualenv for python2.7 - following the other patterns, something like

  - if [[ $TOXENV == py2* ]]; then pip install --upgrade pip virtualenv; fi

(I suspect the bash -c "..." wrapper isn't necessary, but that's a separate thing that could be simplified later I guess.)

But anyway, no significant objection to doing it your way, that's also fine.

EDIT: I guess travis-ci might use /bin/sh for these script lines, which is "dash" instead of "bash" on debian and ubuntu, and that's why bash -c "..." is needed for the [[ ... ]] expressions which are bash features not posix ... nvm

deniszh commented 4 years ago

@ploxiln: Maybe you're right. Let's stick to 2.7 for now.

ploxiln commented 4 years ago

looks good, thanks :) 👍

deniszh commented 4 years ago

Will merge that for now and trigger tests in all non-merged PRs.