Skip to content

Refactor a test to not use the "working_env" fixture#41308

Merged
haampie merged 2 commits intospack:developfrom
alalazo:ci/refactor-a-flakey-test
Nov 29, 2023
Merged

Refactor a test to not use the "working_env" fixture#41308
haampie merged 2 commits intospack:developfrom
alalazo:ci/refactor-a-flakey-test

Conversation

@alalazo
Copy link
Copy Markdown
Member

@alalazo alalazo commented Nov 29, 2023

This test is flakey, and fails spuriously in CI since a few days. See for instance:

https://github.com/spack/spack/actions/runs/7019642159/job/19097775656

Given the comment in the "working_env" fixture:

@pytest.fixture
def working_env():
saved_env = os.environ.copy()
yield
# os.environ = saved_env doesn't work
# it causes module_parsing::test_module_function to fail
# when it's run after any test using this fixutre
os.environ.clear()
os.environ.update(saved_env)

it has been refactored to use monkeypatch. 🤞 we won't see spurious failure afterwards.

@spackbot-app spackbot-app bot added core PR affects Spack core functionality tests General test capability(ies) labels Nov 29, 2023
@haampie
Copy link
Copy Markdown
Member

haampie commented Nov 29, 2023

I reverted a change to this test in my PR.

Can you make it such that module(...) accepts a kwarg environb: Optional[MutableMapping[...]] = None and initialize it as environb = environb or os.environb and then mutate that?

That way you can unit test it without side effects

@alalazo
Copy link
Copy Markdown
Member Author

alalazo commented Nov 29, 2023

Nice, this fails on #41306 🥲

@haampie haampie force-pushed the ci/refactor-a-flakey-test branch from 762822e to 8e93e81 Compare November 29, 2023 11:06
t.write(content)
yield tmpdir

# Once done, cleanup the directory
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

not directly related but well...

@haampie
Copy link
Copy Markdown
Member

haampie commented Nov 29, 2023

This PR is still fine, but #41342 fixes the underlying issue instead of working around it.

@haampie haampie merged commit 889b729 into spack:develop Nov 29, 2023
@alalazo alalazo deleted the ci/refactor-a-flakey-test branch November 30, 2023 05:17
@alalazo alalazo mentioned this pull request Jan 2, 2024
36 tasks
alalazo added a commit that referenced this pull request Jan 2, 2024
alalazo added a commit that referenced this pull request Jan 10, 2024
haampie added a commit that referenced this pull request Jan 11, 2024
RikkiButler20 pushed a commit to RikkiButler20/spack that referenced this pull request Jan 31, 2024
vjranagit pushed a commit to vjranagit/spack that referenced this pull request Jan 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core PR affects Spack core functionality tests General test capability(ies) utilities

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants