Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

test: fix broken test if user had config files#2173

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
nejch merged 1 commit intomainfromjlvillal/config_test_fix
Jul 23, 2022

Conversation

JohnVillalovos
Copy link
Member

Usemonkeypatch to ensure that no config files are reported for the
test.

Closes:#2172

nejch reacted with thumbs up emoji
@JohnVillalovosJohnVillalovos marked this pull request as draftJuly 23, 2022 19:28
@JohnVillalovosJohnVillalovos marked this pull request as ready for reviewJuly 23, 2022 19:34
@JohnVillalovos
Copy link
MemberAuthor

PR#2174 demonstrates the issue.

Use `monkeypatch` to ensure that no config files are reported for thetest.Closes:#2172
Copy link
Member

@nejchnejch left a comment

Choose a reason for hiding this comment

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

Thanks for the catch@JohnVillalovos! We already have some scaffolding for mocking/prepping a clean config environment but perhaps it was not broad enough in where we placed it. It seems we should have a clean environment forall tests by adding that to the roottests/conftest.py e.g.:

  1. Move thismock_clean_env fixture to the root conftest withautouse=True - no testever should break from a user-suppliedPYTHON_GITLAB_CFG:

    @pytest.fixture
    defmock_clean_env(monkeypatch):
    monkeypatch.delenv("PYTHON_GITLAB_CFG",raising=False)

  2. Move more logic to that fixture that we already use to ensure an empty list of default files - same here, no testever should be affected by presence of local files:

    withmonkeypatch.context()asm:
    m.setattr(config,"_DEFAULT_FILES", [])

  3. In any test that does specifically need that, override it again if needed (I think we mostly don't, as we explicitly define things) like we already do here:

    monkeypatch.setenv("PYTHON_GITLAB_CFG","/some/path")

More approaches are discussed herehttps://stackoverflow.com/questions/38748257/disable-autouse-fixtures-on-specific-pytest-marks in case the simple override is not enough.

WDYT? This would then also be more local/CI-agnostic and consistent across all runs and we can stop worrying about config even before we hit CI.

@JohnVillalovos
Copy link
MemberAuthor

That sounds reasonable but I can't help but think this should be merged now and the improvements you mentioned be done afterwards. As at the moment tests are broken for anyone who has a config.

I was going to work on arrays tomorrow and then could try to take a stab at your ideas.

@nejch
Copy link
Member

Sounds good also! I'll merge this for now.

@nejchnejch merged commit1ecbc7c intomainJul 23, 2022
@nejchnejch deleted the jlvillal/config_test_fix branchJuly 23, 2022 23:05
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@nejchnejchnejch requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Unit tests are failing locally
2 participants
@JohnVillalovos@nejch

[8]ページ先頭

©2009-2025 Movatter.jp