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

Run tests in /tmp #374

Merged
merged 5 commits into from
Nov 24, 2022
Merged

Run tests in /tmp #374

merged 5 commits into from
Nov 24, 2022

Conversation

jorisroovers
Copy link
Owner

run tests in /tmp to avoid gitlint picking up the .gitlint config

This avoids that the tests pick up the .gitlint config or information
from gitlint's git config.
Removing .git dir should also no longer be neccessary.
@webknjaz
Copy link
Contributor

FTR if you were to use pytest, it'd provide built-in capabilities to use unique tmp dirs for testing via its fixtures like tmp_path. This way, you could achieve isolation that spreads beyond just CI and would work the same on the contributors' machines too.

@jorisroovers
Copy link
Owner Author

This way, you could achieve isolation that spreads beyond just CI and would work the same on the contributors' machines too.

You're absolutely right about this, I was being hasty here. Let me re-work this.

FWIW, we only use pytest to run tests today, we have no code-level dependency on it in our tests. For consistency reasons, I'm going to keep it this way - I have nothing against writing pytest-style tests (I do so in plenty of other projects), it's just that gitlint uses traditional unittest style tests and it will be jarring if we start mixing in pytest constructs at this point. If we want to start using pytest at the code level, it will require a more holistic approach. I don't think there's a strong argument for that at this time.

While I could fix this in ./run_tests.sh, in the context of #378, that's not the right thing to do. I'll have a look at fixing this in the unit tests itself.

This ensures the tests will work as expected both in CI and locally.
Windows doesn't like it when you delete the current working directory.
Missing os.path.realpath conversion, which matters when working with
temp dirs.
@jorisroovers jorisroovers merged commit a5180cc into main Nov 24, 2022
@jorisroovers jorisroovers deleted the run-tests-in-other-dir branch January 23, 2023 10:03
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 participants