Skip to content

Commit

Permalink
Fix testing config monkeypatch for concurrent test flakiness (#5068)
Browse files Browse the repository at this point in the history
* Fix testing config monkeypatch for concurrent test flakiness

The default_testing_config monkeypatching fixture was added in gh-4653
but did not consider "from .config import get_or_merge_config" cases in
which get_or_merge_config is already bound and thus not patched.

* main_build: Construct config via get_or_merge_config

Helps tests with default_testing_config monkeypatch.

* Tests: Don't preset values for get_or_merge_config

Otherwise default values set for testing_config before, e.g.,
environment variable-dependent config can be set in the tests.

Example: tests/cli/test_main_build.py::test_conda_py_no_period failed
since it sets CONDA_PY=36 but testing_config already had set it before.

---------

Signed-off-by: Marcel Bargull <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
  • Loading branch information
mbargull and pre-commit-ci[bot] authored Nov 17, 2023
1 parent 785ba7d commit b2b7ab3
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 24 deletions.
8 changes: 6 additions & 2 deletions conda_build/cli/main_build.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,11 @@

from .. import api, build, source, utils
from ..conda_interface import add_parser_channels, binstar_upload, cc_conda_build
from ..config import Config, get_channel_urls, zstd_compression_level_default
from ..config import (
get_channel_urls,
get_or_merge_config,
zstd_compression_level_default,
)
from ..deprecations import deprecated
from ..utils import LoggingContext
from .actions import KeyValueAction
Expand Down Expand Up @@ -514,7 +518,7 @@ def check_action(recipe, config):

def execute(args):
_parser, args = parse_args(args)
config = Config(**args.__dict__)
config = get_or_merge_config(None, **args.__dict__)
build.check_external()

# change globals in build module, see comment there as well
Expand Down
10 changes: 8 additions & 2 deletions conda_build/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -913,8 +913,9 @@ def __exit__(self, e_type, e_value, traceback):
self.clean(remove_folders=False)


def get_or_merge_config(config, variant=None, **kwargs):
"""Always returns a new object - never changes the config that might be passed in."""
def _get_or_merge_config(config, variant=None, **kwargs):
# This function should only ever be called via get_or_merge_config.
# It only exists for us to monkeypatch a default config when running tests.
if not config:
config = Config(variant=variant)
else:
Expand All @@ -928,6 +929,11 @@ def get_or_merge_config(config, variant=None, **kwargs):
return config


def get_or_merge_config(config, variant=None, **kwargs):
"""Always returns a new object - never changes the config that might be passed in."""
return _get_or_merge_config(config, variant=variant, **kwargs)


def get_channel_urls(args):
channel_urls = args.get("channel") or args.get("channels") or ()
final_channel_urls = []
Expand Down
13 changes: 4 additions & 9 deletions tests/cli/test_main_metapackage.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
# SPDX-License-Identifier: BSD-3-Clause
import json
import os
import sys
from glob import glob

from conda_build.cli import main_metapackage
Expand All @@ -15,8 +14,7 @@ def test_metapackage(testing_config, testing_workdir):
main_metapackage.execute(args)
test_path = glob(
os.path.join(
sys.prefix,
"conda-bld",
testing_config.croot,
testing_config.host_subdir,
"metapackage_test-1.0-0.tar.bz2",
)
Expand All @@ -38,8 +36,7 @@ def test_metapackage_build_number(testing_config, testing_workdir):
main_metapackage.execute(args)
test_path = glob(
os.path.join(
sys.prefix,
"conda-bld",
testing_config.croot,
testing_config.host_subdir,
"metapackage_test_build_number-1.0-1.tar.bz2",
)
Expand All @@ -61,8 +58,7 @@ def test_metapackage_build_string(testing_config, testing_workdir):
main_metapackage.execute(args)
test_path = glob(
os.path.join(
sys.prefix,
"conda-bld",
testing_config.croot,
testing_config.host_subdir,
"metapackage_test_build_string-1.0-frank*.tar.bz2",
)
Expand All @@ -88,8 +84,7 @@ def test_metapackage_metadata(testing_config, testing_workdir):

test_path = glob(
os.path.join(
sys.prefix,
"conda-bld",
testing_config.croot,
testing_config.host_subdir,
"metapackage_testing_metadata-1.0-0.tar.bz2",
)
Expand Down
12 changes: 6 additions & 6 deletions tests/cli/test_main_render.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,14 @@ def test_render_without_channel_fails(tmp_path):
), f"Expected to get only base package name because it should not be found, but got :{required_package_string}"


def test_render_output_build_path(testing_workdir, testing_metadata, capfd, caplog):
def test_render_output_build_path(
testing_workdir, testing_config, testing_metadata, capfd, caplog
):
api.output_yaml(testing_metadata, "meta.yaml")
args = ["--output", testing_workdir]
main_render.execute(args)
test_path = os.path.join(
sys.prefix,
"conda-bld",
testing_config.croot,
testing_metadata.config.host_subdir,
"test_render_output_build_path-1.0-1.tar.bz2",
)
Expand All @@ -82,15 +83,14 @@ def test_render_output_build_path(testing_workdir, testing_metadata, capfd, capl


def test_render_output_build_path_and_file(
testing_workdir, testing_metadata, capfd, caplog
testing_workdir, testing_config, testing_metadata, capfd, caplog
):
api.output_yaml(testing_metadata, "meta.yaml")
rendered_filename = "out.yaml"
args = ["--output", "--file", rendered_filename, testing_workdir]
main_render.execute(args)
test_path = os.path.join(
sys.prefix,
"conda-bld",
testing_config.croot,
testing_metadata.config.host_subdir,
"test_render_output_build_path_and_file-1.0-1.tar.bz2",
)
Expand Down
15 changes: 10 additions & 5 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,14 @@
import conda_build.config
from conda_build.config import (
Config,
_get_or_merge_config,
_src_cache_root_default,
conda_pkg_format_default,
enable_static_default,
error_overdepending_default,
error_overlinking_default,
exit_on_verify_error_default,
filename_hashing_default,
get_or_merge_config,
ignore_verify_codes_default,
no_rewrite_stdout_env_default,
noarch_python_build_age_default,
Expand Down Expand Up @@ -81,13 +81,12 @@ def testing_config(testing_workdir):
def boolify(v):
return v == "true"

result = Config(
testing_config_kwargs = dict(
croot=testing_workdir,
anaconda_upload=False,
verbose=True,
activate=False,
debug=False,
variant=None,
test_run_post=False,
# These bits ensure that default values are used instead of any
# present in ~/.condarc
Expand All @@ -102,6 +101,8 @@ def boolify(v):
exit_on_verify_error=exit_on_verify_error_default,
conda_pkg_format=conda_pkg_format_default,
)
result = Config(variant=None, **testing_config_kwargs)
result._testing_config_kwargs = testing_config_kwargs
assert result.no_rewrite_stdout_env is False
assert result._src_cache_root is None
assert result.src_cache_root == testing_workdir
Expand All @@ -121,11 +122,15 @@ def default_testing_config(testing_config, monkeypatch, request):
return

def get_or_merge_testing_config(config, variant=None, **kwargs):
return get_or_merge_config(config or testing_config, variant, **kwargs)
merged_kwargs = {}
if not config:
merged_kwargs.update(testing_config._testing_config_kwargs)
merged_kwargs.update(kwargs)
return _get_or_merge_config(config, variant, **merged_kwargs)

monkeypatch.setattr(
conda_build.config,
"get_or_merge_config",
"_get_or_merge_config",
get_or_merge_testing_config,
)

Expand Down

0 comments on commit b2b7ab3

Please sign in to comment.