EliahKagan
commented
1 month ago On the negative side, as Git is updated, there might be test-failures due to that, making CI tests less reliable.
I think that should only be expected to happen if both GIX_TEST_IGNORE_ARCHIVES and GIX_TEST_CREATE_ARCHIVES_EVEN_ON_CI are used together, which is why I did not do that on CI.
In that situation, the Git version and usually other details of the system would have to be the same to prevent changes to tracked files. For example, when I run cargo nextest run --all with GIX_TEST_IGNORE_ARCHIVES=1 locally on any system besides Windows (where archive creation is suppressed in gix-testtools), I get a large number of changed archives.
But with only GIX_TEST_CREATE_ARCHIVES_EVEN_ON_CI and not GIX_TEST_IGNORE_ARCHIVES, I believe the only fixture scripts that should be run are those whose associated archives are:
- Excluded by being listed in a
.gitignorefile, in which casegit diffwon't show anything for them because they're ignored, or - Missing or out of date relative to the most recent change of the fixture script, in which case that is the situation we are looking for, or
- Running because a problem in
gix-testtoolsprevents it from identifying an archive or reuses an identity across multiple archives. But that also seems like something that ought to cause a failure. The latter was the cause of #1440 if I understand #1441 correctly.
However, if there are other situations where just the version of Git changing will cause a failure here, then I think a comment or something should be added to warn about it, possibly in the CI workflow where I already have a comment suggesting to regenerate the archive.
Byron
This adds the ability of
gix-testtoolsto generate archives of fixture output even when the environment is detected to be CI. This behavior is turned off by default, but enabled ifGIX_TEST_CREATE_ARCHIVES_EVEN_ON_CIis set. A small amount of refactoring is included to avoid decreasing code readability while making this change.Although there are a few plausible use cases for this, which along with other information are detailed in commit messages, the motivating use case is to let us use CI to find out if generated archives are out of date. This is something that has happened a number of times, and is not always immediately noticed, because it has had to be observed in the state of a repository after running tests locally.
On this project's CI, this newly recognized environment variable is:
testjob.test-fastjobs. Thetest-fastjobs will use existing generated archives if available, becauseGIX_TEST_IGNORE_ARCHIVESis not set for them, so this should only cause more work to be done on CI in situations where there something is wrong.In addition to setting
GIX_TEST_CREATE_ARCHIVES_EVEN_ON_CIfor thetest-fastjobs, this also adds a new step at the end to rungit diff --exit-code. That actually detects if any new archives were create. This is because, in practice, as far as I know, they always end up being at least slightly different from whatever existed before for them, if anything.There may be slightly more efficient ways to detect this, and maybe even slightly more reliable ways to detect it, but I think it makes sense to start with this way, because:
gix-testtoolsever want to use CI to generate their archives and save them as artifacts for reuse.As of now, CI will fail on this PR, because there is an archive that needs to be regenerated. The new step can be expected to fails with:
If #1589 is merged to main, and then this PR is rebased onto main (or main is merged into it afterwards), CI here should pass.