Test fixtures are not cleaned up after a test run and may be stale on subsequent runs

[inferiorhumanorgans]

Per #382 the default behavior with test fixtures is:

• to only regenerate them when the contents of the fixture script changes
• to not remove the fixtures after a test success

Potentially more desirable behavior is:

• automatically remove the generated fixture after a test completes
• a tunable (env var) to allow archiving the fixture before removal
• or always archive before removal

I'm thinking the default behavior should be to leverage TempDir with a check at the end to see if an environment variable is set. If the variable is set, archive the fixture directory before the TempDir object goes out of scope.

Alternatively the existing logic (hashing the contents of the fixture script and placing the output in a directory within the repo) could be retained.

With either implementation the question is whether it's more desirable to move towards putting the test logic into closures (for more automatic cleanup) or to add an explicit check at the end of each test.

Thoughts?

[Byron]

Thanks for sharing your thoughts!
I would love to get a better understanding of the problem changes should solve or a proposition.
Thanks again

[inferiorhumanorgans]

The problem I had specifically is that when doing A/B testing I'd toggle between two different versions of a fixture script (let's say CRC=1 and CRC=2). The problem is that the output is always preserved and because the CRC value never changed beyond 1 or 2 the output got reused for the next test run (creating either false success or false failure depending). My solution was to manually remove the generated-do-not-edit directory between runs, but this violates the policy of least surprise – IMO it's reasonable to assume that each test run will re-run the fixture scripts as necessary.

My proposal is to have git_testtools::scripted_fixture_repo_read_only return a TempDir object similar to what git_testtools::scripted_fixture_repo_writable already does. This solves the cleanup portion of the issue.

For the archival portion I think it's pretty well agreed that archiving the fixture output should be gated on an environment variable (e.g. GITOXIDE_ARCHIVE_TEST=1 cargo test …). The question I have is what implementation of this gate would fit best with gitoxide?

• Add an epilogue to each test that uses fixtures e.g. in git-repository

  #[test]
  fn test_with_fixture() -> Result {
    // prelude
    let repo = repo("fixture_script.sh").map(|r| r.to_thread_local())?;
  
    /* test logic */
  
    // epilogue
    archive_if_needed(repo.path(), "name-of-test-case");
  }

• Create a wrapper struct around TempDir whose Drop impl handles the archival logic. I have a mild preference for this because it should "just work" with few invasive changes .
• Create a function that moves test logic into a fixture e.g.:

  #[test]
  fn test_with_fixture() -> Result {
    // run_fixture will create a temp_dir, run the fixture script, run the closure,
    // and optionally create an archive after the closure is finished
    run_fixture("fixture_script.sh", |repo| {
      let repo = repo("fixture_script.sh").map(|r| r.to_thread_local())?;
      /* test logic */ 
    });
  }