On Thu, Jun 30, 2022 at 3:26 AM Ævar Arnfjörð Bjarmason
<avarab@xxxxxxxxx> wrote:
>
> On Thu, Jun 30 2022, Elijah Newren via GitGitGadget wrote:
>
> > From: Elijah Newren <newren@xxxxxxxxx>
>
> > +test_setup_12l () {
> > + test_create_repo 12l_$1 &&
>
> Can & should just be "git init", see f0d4d398e28 (test-lib: split up and
> deprecate test_create_repo(), 2021-05-10).
I've switched to "git init" and even encouraged others to do the same
in other test scripts, but since literally every other test in this
file uses test_create_repo, I think they should all be converted at
once and just be consistent here. But, so we can stop having this
conversation, after this series lands, I'll send one in to update the
various merge testfiles that make heavy use of test_create_repo and
convert them over.
> > + (
> > + cd 12l_$1 &&
> > +
> > + mkdir -p sub1 sub2
>
> The "-p" here isn't needed, you're not creating recursive directories.
Indeed; will fix.
> > + git ls-files -s >out &&
> > + test_line_count = 5 out &&
>
> Can't this just use test_stdout_line_count? Except...
Ooh, new function from late last year that I was unaware of. Yeah,
I'm happy to start using it.
> > +
> > + git rev-parse >actual \
> > + :0:sub3/file :0:sub3/newfile :0:sub3/sub2/other \
> > + :2:sub1/sub2/new_add_add_file \
> > + :3:sub1/sub2/new_add_add_file &&
> > + git rev-parse >expect \
> > + O:sub1/file B:sub1/newfile O:sub2/other \
> > + A:sub2/new_add_add_file \
> > + B:sub1/sub2/new_add_add_file &&
> > + test_cmp expect actual &&
> > +
> > + git ls-files -o >actual &&
> > + test_write_lines actual expect out >expect &&
> > + test_cmp expect actual
>
> This test seems a bit odd, here we're just checking that we've created
> scratch files for the internal use of our test, why is it important that
> we have an "out" file, when the only reason we have it is because we
> needed it for checking the "ls-files" line count above?
Nah, you've misunderstood the purpose of the check. It isn't "make
sure that these untracked files are present among whatever else might
also be present", it's "make sure the merge step did not introduce
*any* untracked files" (something the recursive backend periodically
did, and they weren't cruft untracked files but stored actual merge
results). There wasn't a nice easy check for that, the closest was to
translate the requirement to "make sure the only untracked files are
the ones explicitly added by this test script", which is the check you
see here. I don't actually care about "actual", "expect", or "out", I
just care that there aren't any _other_ untracked files.
