Skip to content

Cover gix with most features but no extras #1995

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

Merged

Conversation

EliahKagan
Copy link
Member

@EliahKagan EliahKagan commented May 6, 2025

This expands the unit-tests recipe in the justfile (which is one of the recipes the full CI test job runs) to add a command that tests gix with all the current default features except for extras and its subfeatures.

This appears to have been a missing potentially valuable area in testing.

In particular, as noted in 2400158 (#1993), the Git 2.47.* bug that caused incorrect find_youngest_matching_commit baselines to be generated had applied equally to the regex_matches and contained_string_matches tests, but only the regex_matches failure was found locally in #1622 and on CI in #1635, because no combination of features under which contained_string_matches runs was tested.

Thus, contained_string_matches was effectively not exercised, and regressions that it should catch, as well as the effect of possible changes to the test if any others are ever made, were not tested. This resulted in the mitigations for #1622 not covering that test either (until 2400158).

This PR chooses a combination of features that may plausibly be in practical use and that is fairly simple to specify, to fill the gap.

(If this turns out to slow things down too much, then it could be narrowed to only run some tests.)

This only adds a command to the recipe. It does not remove any commands that were already there. gix remains tested in all the ways it was tested before, as well as in this one new way.


I'm waiting to look at the timings before merging this.

Edit: That seems fine--performance seems not to be significantly affected. But now I am checking whether this change causes modified tracked .tar fixture archives to be created when run locally (or if that was happening already with just unit-tests).

Edit 2: This does not introduce that--it already happens. Locally, on both Ubuntu 24.04 LTS and macOS 15.4.1, running just unit-tests on main without GIX_TEST_IGNORE_ARCHIVES adds two untracked files (with GIX_TEST_IGNORE_ARCHIVES, there would intentionally be many more archives modified, though still not any untracked unignored archives created):

ubuntu@crow:~/repos/gitoxide (main %=)$ git status
On branch main
Your branch is up to date with 'origin/main'.

Untracked files:
  (use "git add <file>..." to include in what will be committed)
        gix-odb/tests/fixtures/generated-archives/make_repo_multi_index_no.tar
        gix-odb/tests/fixtures/generated-archives/make_repo_multi_index_without-multi-index.tar

nothing added to commit but untracked files present (use "git add" to track)

Those are the same as with the changes here. So even if that is undesirable, this doesn't introduce it and doesn't seem to worsen it, and thus it need not block this PR.

I'm going to check another couple things locally before merging this, though.

Edit 3: Unrelated to the above concerns, I've verified that GIX_TEST_IGNORE_ARCHIVES=1 just unit-tests works locally on Ubuntu 24.04 LTS and macOS 15.4.1, and I've also rebased to slightly clarify the commit message.

This expands the `unit-tests` recipe in the `justfile` (which is
one of the recipes the full CI `test` job runs) to add a command
that tests `gix` with all the current default features *except* for
`extras` and its subfeatures.

This appears to have been a missing potentially valuable area in
testing.

In particular, as noted in 2400158 (GitoxideLabs#1993), the Git 2.47.* bug that
caused incorrect `find_youngest_matching_commit` baselines to be
generated had applied equally to the `regex_matches` and
`contained_string_matches` tests, but only the `regex_matches`
failure was found locally in GitoxideLabs#1622 and on CI in GitoxideLabs#1635, because no
combination of features under which `contained_string_matches` runs
was tested.

Thus, `contained_string_matches` was effectively not exercised, and
regressions that it should catch, as well as the effect of possible
changes to the test if any others are ever made, were not tested.
This resulted in the mitigations for GitoxideLabs#1622 not covering that test
either (until 2400158).

This commit chooses a combination of features that may plausibly be
in practical use and that is fairly simple to specify, to fill the
gap.

(If this turns out to slow things down too much, then it could be
narrowed to only run some tests.)

This only adds a command to the recipe. It does not remove any
commands that were already there. `gix` remains tested in all the
ways it was tested before, as well as in this one new way.
@EliahKagan EliahKagan force-pushed the run-ci/test-without-extras branch from 1c72063 to 7be6b7f Compare May 6, 2025 00:43
@EliahKagan EliahKagan marked this pull request as ready for review May 6, 2025 00:55
@EliahKagan EliahKagan enabled auto-merge May 6, 2025 00:55
@EliahKagan EliahKagan merged commit 1163938 into GitoxideLabs:main May 6, 2025
21 checks passed
@EliahKagan EliahKagan deleted the run-ci/test-without-extras branch May 6, 2025 01:00
@Byron
Copy link
Member

Byron commented May 6, 2025

Thank you!

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