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

[Improvement] Add an integration test for checking that PRs do not leave unchecked files behind #3317

Open
3 tasks done
dianpopa opened this issue Dec 12, 2022 · 4 comments
Open
3 tasks done
Assignees
Labels
Good first issue Indicates a good issue for first-time contributors Status: Parked Indicates that an issues or pull request will be revisited later Type: Enhancement Indicates new feature requests

Comments

@dianpopa
Copy link
Contributor

Describe the bug

We have been having 2 cases in the last 2 weeks where tests/host_tools/uffd/Cargo.lock was left behind and not checked in as part of the PR changes that dirtied it.

Would be nice to avoid this situations by implementing a integ test for ensuring that git does not report any dirty file.

Checks

  • Have you searched the Firecracker Issues database for similar problems?
  • Have you read the existing relevant Firecracker documentation?
  • Are you certain the bug being reported is a Firecracker issue?
@dianpopa dianpopa added Type: Bug Indicates an unexpected problem or unintended behavior Quality: Improvement and removed Type: Bug Indicates an unexpected problem or unintended behavior labels Dec 12, 2022
@pb8o pb8o self-assigned this Dec 27, 2022
@pb8o pb8o added the Good first issue Indicates a good issue for first-time contributors label Dec 27, 2022
@pb8o
Copy link
Contributor

pb8o commented Dec 27, 2022

There is already a similar test here: https://github.com/firecracker-microvm/firecracker/blob/main/tests/integration_tests/style/test_repo.py Should be easy to extend.

@pb8o
Copy link
Contributor

pb8o commented Dec 27, 2022

However, the test would have to run last, and I think that's not easy to do with pytest.

@roypat
Copy link
Contributor

roypat commented Feb 17, 2023

Apparently it would be possible to use an "autouse" fixture that has an assert for this in its teardown logic: https://stackoverflow.com/questions/22627659/run-code-before-and-after-each-test-in-py-test

@pb8o
Copy link
Contributor

pb8o commented Feb 22, 2023

Ah yes, that's a good approach. Something like this should work then

@pytest.fixture(autouse=True, scope="session")
def check_nothing_added_after_all_tests_run():
    yield None
    # check that git working copy is clean
    assert check_output("git status --porcelain") == ""

@JonathanWoollett-Light JonathanWoollett-Light added Type: Enhancement Indicates new feature requests and removed Quality: Improvement labels Mar 24, 2023
@zulinx86 zulinx86 added the Status: Parked Indicates that an issues or pull request will be revisited later label Jun 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good first issue Indicates a good issue for first-time contributors Status: Parked Indicates that an issues or pull request will be revisited later Type: Enhancement Indicates new feature requests
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants