Skip to content
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

Custom naming for previous requirements hash file #10

Closed
dgilland opened this issue Aug 10, 2017 · 5 comments
Closed

Custom naming for previous requirements hash file #10

dgilland opened this issue Aug 10, 2017 · 5 comments
Assignees
Labels

Comments

@dgilland
Copy link

For some of my tox projects, I use a single virtualenv for all test commands. The config looks something like this:

[tox]
envlist = flake8, pylint, docs, unit

[testenv]
envdir = {toxworkdir}/env
deps =
    -rrequirements-test.txt
commands =
    flake8,lint,test: flake8
    pylint,lint,test: pylint
    docs: sphinx-build
    unit,test: pytest

So even though I have different tox environments, they all share the same virtualenv.

What I run into with tox-battery is that the initial tox build ends up recreating each tox env for a single run of $ tox because there doesn't exist a requirements.previous file yet for each of the envs. Subsequent runs are fine though. For my local dev side, this isn't too much of a problem; however, for CI servers, the build times are longer than they need to be due to the venv recreation since the CI build always starts with a fresh checkout.

My question is whether you would consider having an option to name the previous requirements file based on the actual venv path instead of the tox env name or possibly a setting under the env configuration that explicitly defines the name used in previous requirements file. Something like:

[testenv:flake8]
# would be used to build filename "requirements.txt.{tox_battery_basename}.previous"
tox_battery_basename = env

[testenv:pylint]
tox_battery_basename = env

and maybe a global option which would apply to all envs unless they overwrote the value in the env specific config:

[testenv]
tox_battery_basename = env

Thoughts? If you are open to this idea, I could submit a PR for it.

Thanks!

signalpillar pushed a commit that referenced this issue Aug 12, 2017
Problem
-------
It is not supported reuse of venv between multiple testenvs.
For instance::

    [tox]
    envlist = flake8, pylint, docs, unit

    [testenv]
    envdir = {toxworkdir}/env
    deps =
        -rrequirements-test.txt
    commands =
        flake8,lint,test: flake8
        pylint,lint,test: pylint
        docs: sphinx-build
        unit,test: pytest

In this case user set 'envdir' that makes all the registered testenvs share the
same venv. In this case tox-bat will recreate same venv for each testenv as
'requirements-test.txt' file belongs to testenv not venv.
In name of the .previous file we use testenv name and if file doesn't exist -
environment needs to be recreated.

Solution
--------
Change name of the .previous file to contain SHA1 of venv path used by testenv
instead of its name.
Normally when user doesn't change 'envdir' manually, each testenv has own venv
and own .previous file.
When user sets 'envdir' to be shared between some testenvs they will share the
same .previous file.

Other changes
-------------
Small fix that sorts the parsed requirements and reshuffling or commenting
doesn't cause recreating of the venv.

Testing
-------
Added test for this particular case that checks venv is reused between two
testenvs.

- Added new fixtures to work with temporary project
- Removed obsolete test case.
- Fixed some typoes.
@signalpillar
Copy link
Owner

signalpillar commented Aug 12, 2017

Hi Derrick ( @dgilland ), thank you for this well formed issue. It is a really interesting case that I've never considered myself and was happy to address.

As a result (PR #12 ) I managed to support case with custom envdir without modifying client's tox.ini file.

Could you please look review that PR and tell me whether it looks reasonable for you. Especially if you can give me advice how to improve description of file name format for .previous . I'll try to merge it this weekend after playing with it a bit.

Thank you!

@signalpillar signalpillar self-assigned this Aug 12, 2017
signalpillar pushed a commit that referenced this issue Aug 12, 2017
Problem
-------
It is not supported reuse of venv between multiple testenvs.
For instance::

    [tox]
    envlist = flake8, pylint, docs, unit

    [testenv]
    envdir = {toxworkdir}/env
    deps =
        -rrequirements-test.txt
    commands =
        flake8,lint,test: flake8
        pylint,lint,test: pylint
        docs: sphinx-build
        unit,test: pytest

In this case user set 'envdir' that makes all the registered testenvs share the
same venv. In this case tox-bat will recreate same venv for each testenv as
'requirements-test.txt' file belongs to testenv not venv.
In name of the .previous file we use testenv name and if file doesn't exist -
environment needs to be recreated.

Solution
--------
Change name of the .previous file to contain SHA1 of venv path used by testenv
instead of its name.
Normally when user doesn't change 'envdir' manually, each testenv has own venv
and own .previous file.
When user sets 'envdir' to be shared between some testenvs they will share the
same .previous file.

Other changes
-------------
Small fix that sorts the parsed requirements and reshuffling or commenting
doesn't cause recreating of the venv.

Testing
-------
Added test for this particular case that checks venv is reused between two
testenvs.

- Added new fixtures to work with temporary project
- Removed obsolete test case.
- Fixed some typoes.
@dgilland
Copy link
Author

I don't really have any other suggestions on naming the .previous file. A hash of the envdir seems sufficient. Only downside is that it's not obvious which env it applies to if someone were to look at the files (not sure if that's even something to consider).

@signalpillar
Copy link
Owner

Yeah, that is true about file names, they are quite cryptic. From other side, I am not sure somebody has to read them. At least, so far. Thanks again for you review! I've addressed your latest finding.

@dgilland
Copy link
Author

Awesome work! Thanks!

@signalpillar
Copy link
Owner

@dgilland thank you for you review and help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants