Skip to content

refactor config tests to control HOME environment variable#5455

Merged
outofambit merged 5 commits intomasterfrom
config-test-depends-on-HOME
Aug 27, 2018
Merged

refactor config tests to control HOME environment variable#5455
outofambit merged 5 commits intomasterfrom
config-test-depends-on-HOME

Conversation

@shiftkey
Copy link
Copy Markdown
Member

The Jest PR #5427 identified some tests that touch the global config, and this is suboptimal for a couple of reasons:

  • inconsistent setup between CI environments - not every environment provides it
  • tests should not be modifying the user's global config as a result of running tests

This PR refactors the config.ts to support controlling the HOME environment variable, which Git should be using to read the global config. This also updates the config-tests.ts that touch the global config to now use a temporary directory.

@shiftkey shiftkey added ready-for-review Pull Requests that are ready to be reviewed by the maintainers infrastructure Issues and pull requests related to scripts and tooling for GitHub Desktop labels Aug 23, 2018
@shiftkey shiftkey requested a review from outofambit August 24, 2018 12:51
Copy link
Copy Markdown
Contributor

@outofambit outofambit left a comment

Choose a reason for hiding this comment

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

Looks great! Thank you!

@outofambit outofambit self-assigned this Aug 24, 2018
@outofambit outofambit merged commit 645fbc4 into master Aug 27, 2018
@outofambit outofambit deleted the config-test-depends-on-HOME branch August 27, 2018 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

infrastructure Issues and pull requests related to scripts and tooling for GitHub Desktop ready-for-review Pull Requests that are ready to be reviewed by the maintainers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants