1601737 - Replace global "fresh_global_config" fixture with context manager

Status: NEW

(Conduit :: moz-phab, enhancement, P3)

Description

[Mitchell Hentges [:mhentges] 🦀]

When moz-phab test run right now, they depend on a fixture to override the global system-wide git and hg settings with more controlled test values.

However, this approach has some downsides:

• It's not obvious where the overridden config is coming from
• What if just one test needs the .hgrc tweaked in a special way? It's hard to have a test-specific config without overriding the override
• Not all tests need this fixture. As more are added, this slowly bogs down our test suite, adding unnecessary delays

Instead, I think we should have a nice context manager like so:

def test_juicers(tmp_path):
    with init_hg_repo(tmp_path):
        # Overrides `env["HOME"]` to point to tmp_path, sets `.hgrc`
        ...

def test_sauce(tmp_path):
    with init_git_repo(tmp_path):
        # Overrides `env["home"]` to point to tmp_path, sets `.gitconfig`
        ...

Benefits:

• Set-ups for different VCS systems are now contextual, so there isn't a global "set everything up for everyone" function
• You can easily tell what tests are affected by what set-up logic
• In-IDE navigation is easier (control-click on init_git_repo, can immediately see what's going on)

Comment 1

[Mitchell Hentges [:mhentges] 🦀]

I don't think that this is a high-priority situation to resolve, but I'd be interested in hearing what :glob and :zalun think. CC-ing :)

Comment 2

[Mitchell Hentges [:mhentges] 🦀]

Also, if you wanted to get really fancy with this, you could improve testing ergonomics at the same time with helper functions:

def test_juicers(tmp_path):
    with init_hg_repo(tmp_path) as repo:
        repo.add_commit('a')
        repo.add_commit('b')
        repo.hg_specific_helper_thing(params)
        ...

Comment 3

[Piotr Zalewa [:zalun]]

The next step is to refactor moz-phab into modules. This will come along with refactoring the tests.
Using with creates another block that would be used for the entire test.
If this could be solved using a decorator it would save us some left-hand space.

Comment 4

[Mitchell Hentges [:mhentges] 🦀]

If this could be solved using a decorator it would save us some left-hand space.

Ooh, yeah, that's a good point. A test-local fixture could also work :)