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

#595 fix by updating implementation of #515 #598

Closed
wants to merge 2 commits into from
Closed

#595 fix by updating implementation of #515 #598

wants to merge 2 commits into from

Conversation

obestwalter
Copy link
Member

@obestwalter obestwalter commented Sep 2, 2017

fixes #595

The way #515 was implemented did not play nicely with env var substitution. This might render the former additions completely or partially obsolete but I did not check this yet. First let's fix that bug.

The way #515 was implemented did not play nicely with env var substitution. This might render the former additions completely or partially obsolete but I did not check this yet. First let's fix that bug.
@obestwalter
Copy link
Member Author

obestwalter commented Sep 2, 2017

Hi @AndreLouisCaron, could you also have a look at this?

@@ -791,9 +791,8 @@ def __init__(self, config, inipath):
section = testenvprefix + name
factors = set(name.split('-'))
if section in self._cfg or factors <= known_factors:
config.envconfigs[name] = \
self.make_envconfig(name, section, reader._subs, config,
replace=name in config.envlist)
Copy link
Member Author

@obestwalter obestwalter Sep 2, 2017

Choose a reason for hiding this comment

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

This list only contains the envs in envlist and would mean that only testenvs get substituted that are defined in envlist. This is why we always replace at this early stage (configuration parsing) and don't crash if something is missing in a testenv. When an actual testrun is started we can check if a testenv would be run that has missing substitutions.

(cherry picked from commit e902f04)
@obestwalter obestwalter requested review from The-Compiler and removed request for The-Compiler September 2, 2017 20:08
@obestwalter
Copy link
Member Author

This is not the right way to fix this. I will open another one.

@obestwalter obestwalter closed this Sep 3, 2017
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.8.0: {[testenv]setenv} substitution broken
1 participant