change the HOME for git commands#649
change the HOME for git commands#649gaborbernat wants to merge 3 commits intopre-commit:masterfrom gaborbernat:master
Conversation
| @pytest.fixture() | ||
| def setup_git(monkeypatch, tmpdir): | ||
| # we should not use user configuration | ||
| monkeypatch.setenv('HOME', tmpdir) |
There was a problem hiding this comment.
we had the same situation in pre-commit-hooks and I think this is the wrong solution as it has the potential to hide problems with different configurations. We instead opted to pass --no-gpg-sign to any git commits under test: pre-commit/pre-commit-hooks#121
There was a problem hiding this comment.
this fixture also doesn't scale (adding it to literally every test) -- perhaps instead an autouse fixture would have been more appropriate
| assert _norm_out(ret[1]) == expected | ||
| @pytest.fixture(name='test_hook_repo') | ||
| def test_hook_repo_fixture(tempdir_factory, store, setup_git): | ||
| def do(repo_path, |
| skipif_cant_run_bash = pytest.mark.skipif( | ||
| parse_shebang.find_executable('bash') is None, | ||
| reason='bash isn\'t installed or can\'t be found', | ||
| ) |
There was a problem hiding this comment.
without bash, most of the testsuite is useless -- pre-commit tests lots of the framework functionality with script hooks as they are fast. I don't think I can accept a PR which adds skips for bash. As bash is necessarily installed on all the platforms we support (it's necessary for git itself to function) this also seems inappropriate.
|
|
||
| for path in paths: | ||
| if os.path.exists(path): | ||
| os.rmdir(path) |
|
@asottile I did not knew bash is required. I'm personally not comfortable knowing this to add this project to tox; as you make it at best cumbersome for people on Windows to develop; now they need to install some kind of bash beside just Python. |
|
It's not required for runtime, just very important for the test suite. Also if you're running git on windows, you're running bash ;) |
|
@asottile I'm using cmder which includes bash, however the test fails as it tries to find |
|
Can you produce the output of the test as well as |
|
this avoids using the users configuration, which can contain e.g. gpg signing with password which would break this tests