-
Notifications
You must be signed in to change notification settings - Fork 34
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
Move toxenv to tox_configure hook #78
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -416,7 +416,7 @@ def test_travis_ignore_outcome(self, tmpdir, monkeypatch): | |
"""Test ignore_outcome without setting obey_outcomes.""" | ||
tox_ini = tox_ini_ignore_outcome | ||
with self.configure( | ||
tmpdir, monkeypatch, tox_ini, travis_language='generic' | ||
tmpdir, monkeypatch, tox_ini, travis_version='3.4' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Out of curiosity, what was the reason behind this change? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, that makes sense. |
||
): | ||
config = self.tox_config() | ||
assert config["testenv:py34"]["ignore_outcome"] == "True" | ||
|
@@ -426,7 +426,7 @@ def test_travis_ignore_outcome_unignore_outcomes(self, tmpdir, | |
"""Test ignore_outcome setting unignore_outcomes = False.""" | ||
tox_ini = tox_ini_ignore_outcome_unignore_outcomes | ||
with self.configure( | ||
tmpdir, monkeypatch, tox_ini, travis_language='generic' | ||
tmpdir, monkeypatch, tox_ini, travis_version='3.4' | ||
): | ||
config = self.tox_config() | ||
assert config["testenv:py34"]["ignore_outcome"] == "False" | ||
|
@@ -436,7 +436,7 @@ def test_travis_ignore_outcome_not_unignore_outcomes(self, tmpdir, | |
"""Test ignore_outcome setting unignore_outcomes = False (default).""" | ||
tox_ini = tox_ini_ignore_outcome_not_unignore_outcomes | ||
with self.configure( | ||
tmpdir, monkeypatch, tox_ini, travis_language='generic' | ||
tmpdir, monkeypatch, tox_ini, travis_version='3.4' | ||
): | ||
config = self.tox_config() | ||
assert config["testenv:py34"]["ignore_outcome"] == "True" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part puzzles me, comparing this to #61, I don't understand what exactly
autgen_envconfigs
does and why it's necessary. It's not clear to me what's depending on the generated envconfigs, and why this wasn't necessary in #61.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a case that #61 didn't cover, and exists for backward compatibility with the first release of tox-travis. With that first release, you would declare what environments got run for each version of Python. It didn't look at the environments that were listed at all, it just sent the string in the configuration verbatim, through the TOXENV environment variable. If the specified environment is not declared in the tox config, Tox will still happily run that environment, doing the best it can based on the settings it has.
When we switched to matching based on factors, so that by default on Python 3.4 all envs with
py34
in the name would run, that obviously was inherently in conflict with the original mechanism. However, I was able to maintain partial backward compatibility by saying that if there was only one "factor" being matched (python
, which is automatic), and no environments matched it, then whatever the setting is will be used verbatim, in the same way as was done in the original version.This is a wart, but fixing it means a breaking change. This workaround allows me to get this change in without having to worry about a breaking change at this time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, and as far as what it actually does. Before the tox_configure hook, Tox looks at all the environments, and creates a envconfig for each one. The envconfig is the configuration of how to run the environment, with default settings resolved. What this does is notice when we've ended up with an envconfig that Tox didn't know about to begin with, and make an envconfig for it the same way Tox would have if it had been specified in
TOXENV
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha, this is compatibility code, and would presumably not be present in the next major release.