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

test: use isolated, temporary GIT_CONFIG #420

Merged
merged 3 commits into from
Oct 22, 2021

Conversation

rwe
Copy link
Contributor

@rwe rwe commented Oct 21, 2021

I noticed that the unit tests were changing the configuration in .git/config. They also read user configuration as fallback values, which made things verifying correct behaviour confusing—especially when some user configuration matches the defaults!

(Although there was a specific guard checking against in_unit_test, that was only for boolean config values, not configuration in general).

This generates a gitconfig in $BATS_TMPDIR and exports both GIT_CONFIG=… and GIT_CONFIG_NOSYSTEM=1, which git will respect for isolating that configuration.

That setup and teardown is done in setup_file (once per suite, rather than once per test). Some other test setup has been moved into those bat callbacks as well.

@rwe
Copy link
Contributor Author

rwe commented Oct 22, 2021

I'll rebase this on common base of master and next and point to next.

rwe added 3 commits October 22, 2021 11:59
This writes a temp git config file during setup/teardown, and exports
GIT_CONFIG=… to that and sets GIT_CONFIG_NOSYSTEM=1. This ensures that
any git invocations _only_ use that configuration.

Note that none of the tests currently test anything other than the
default config; however, if they do, the generation of this test config
should be moved into `setup()` rather than `setup_file()` so that it can
be modified and restored between tests.
@rwe rwe changed the base branch from master to next October 22, 2021 19:00
@rwe
Copy link
Contributor Author

rwe commented Oct 22, 2021

Done; this now targets next.

@scottchiefbaker
Copy link
Contributor

This looks cool. I've been wanting to implement something like this for a while, but never had the time. I don't totally understand all of it, so if something breaks down the line (the tests all pass right now) I may have to poke you to help fix things.

Great work.

@scottchiefbaker scottchiefbaker merged commit 3c4fb51 into so-fancy:next Oct 22, 2021
@scottchiefbaker
Copy link
Contributor

Well... maybe I spoke too soon? I merge it, and now some tests are failing?

@rwe
Copy link
Contributor Author

rwe commented Oct 23, 2021

I'm not seeing the failure, do you have a link?

@scottchiefbaker
Copy link
Contributor

@rwe false alarm I guess. My local branch had failing tests. When I did a clean checkout all the tests pass. I must have messed up my local branch somehow. We're good.

Nothing to see here. Move along :)

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