| Message ID | ccc26a8950be41e5be4dc78295c66ecbade8a50e.1618829583.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Prepare tests for reftable backend | expand |
On Mon, Apr 19 2021, Han-Wen Nienhuys via GitGitGadget wrote: > From: Han-Wen Nienhuys <hanwen@google.com> > > Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> > --- > t/t9300-fast-import.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh > index 5c47ac4465cb..087ddf097036 100755 > --- a/t/t9300-fast-import.sh > +++ b/t/t9300-fast-import.sh > @@ -392,7 +392,7 @@ test_expect_success 'B: accept branch name "TEMP_TAG"' ' > git gc > git prune" && > git fast-import <input && > - test -f .git/TEMP_TAG && > + git rev-parse TEMP_TAG && > test $(git rev-parse main) = $(git rev-parse TEMP_TAG^) > ' It seems to me that this breaks the test, to the extent that it's testing something we care about at all. I.e. reading ea08a6fd19 (Actually allow TAG_FIXUP branches in fast-import, 2007-08-02) the whole point is to test that a "TEMP_TAG" ref is accepted by fast-import, as opposed to "refs/heads/TEMP_TAG". So we're trying to check that the refstore accepts a concept of a name like HEAD called BLAHBLAH or whatever, that lives at the "top-level". For something unrelated I was trying to figure out how to detect that the other day and couldn't come up with anything I was happy with, e.g. this seems to work: # Rely on for-each-ref not listing it... git for-each-ref >gfe && ! grep TEMP_TAG gfe && # ... but rev-parse grokking it git rev-parse TEMP_TAG Or, rely on rev-parse emitting a warning about it: $ git update-ref refs/heads/TEMP_TAG HEAD # A bug? We emit both an error and a warning with # --symbolic-full-name? $ git rev-parse --symbolic-full-name TEMP_TAG warning: refname 'TEMP_TAG' is ambiguous. error: refname 'TEMP_TAG' is ambiguous $ echo $? 0 IOW re my https://lore.kernel.org/git/87wnt2th1v.fsf@evledraar.gmail.com/ this seems like exactly the sort of edge case where we want to dig a bit more and assert that we support (or explicitly don't support) this "mode" under reftable. In any case, if we're not testing for .git/TEMP_TAG anymore I think your addition of "git rev-parse TEMP_TAG" is entirely redundant to the ad-hoc "test_cmp" that comes after it, when would we not be able to rev-parse FOO, but succeed with FOO^ ?
On Wed, Apr 21, 2021 at 8:01 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > It seems to me that this breaks the test, to the extent that it's > testing something we care about at all. > > I.e. reading ea08a6fd19 (Actually allow TAG_FIXUP branches in > fast-import, 2007-08-02) the whole point is to test that a "TEMP_TAG" > ref is accepted by fast-import, as opposed to "refs/heads/TEMP_TAG". I've changed it to use the test-tool instead, which doesn't search for refs.
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh index 5c47ac4465cb..087ddf097036 100755 --- a/t/t9300-fast-import.sh +++ b/t/t9300-fast-import.sh @@ -392,7 +392,7 @@ test_expect_success 'B: accept branch name "TEMP_TAG"' ' git gc git prune" && git fast-import <input && - test -f .git/TEMP_TAG && + git rev-parse TEMP_TAG && test $(git rev-parse main) = $(git rev-parse TEMP_TAG^) '