On Wed, Feb 26, 2020 at 7:13 PM Junio C Hamano <gitster@xxxxxxxxx> wrote:
> > diff --git a/t/t0031-reftable.sh b/t/t0031-reftable.sh
> > new file mode 100755
> > index 00000000000..d35211c9db2
> > --- /dev/null
> > +++ b/t/t0031-reftable.sh
Thanks for your careful review. Should we not start with the C code,
though? I've written the reftable library to my best ability, but it
hasn't been peer reviewed yet.
> > +test_expect_success 'basic operation of reftable storage' '
> > + git init --ref-storage=reftable repo &&
>
> Test script bodies are supposed to be indented with HT, not 8 SPs.
Done
> > + cd repo &&
>
> You are not supposed to chdir around inside tests, unless you do so
> in a subshell. i.e.
>
> test_expect_success 'basic operation' '
> git init --ref-storage=reftable repo &&
> (
> cd repo &&
Done.
> > + echo "hello" > world.txt &&
>
> A shell redirection operator should immediately be followed by the
> filename without SP in between, i.e.
Done.
> echo hello >world.txt &&
>
> > + git add world.txt &&
> > + git commit -m "first post" &&
> > + printf "HEAD\nrefs/heads/master\n" > want &&
>
> There is an easier-to-read helper. Also, by convention, the file
> that holds the expected state is called "expect", while the file
> that records the actual state observed is called "actual".
Done
> test_write_lines HEAD refs/heads/master >expect &&
>
> > + git show-ref | cut -f2 -d" " > got &&
>
> Is the order of "show-ref" output guaranteed in some way? Otherwise
> we may need to make sure that we sort it to keep it stable.
The ref iteration result is lexicographically ordered.
> Without --show-head, HEAD should not appear in "git show-ref"
> output, but the expected output has HEAD, which is puzzling.
The HEAD symref is stored in reftable too, and to the reftable code
it's just another ref. How should we refactor this? Shouldn't the
files ref backend produce "HEAD" and then it's up to the show-ref
builtin to filter it out?
> We will not notice if a "git" command that placed on the left hand
> side of a pipeline segfaults after showing its output.
>
> People often split a pipeline like this into separate command for
> that reason (but there is probably a better way in this case, as we
> see soon).
>
> > + test_cmp got want &&
>
> Taking the above comments together, perhaps
>
> test_write_lines refs/heads/master >expect &&
> git for-each-ref --sort=refname >actual &&
> test_cmp expect actual &&
>
> > + for count in $(test_seq 1 10); do
> > + echo "hello" >> world.txt
> > + git commit -m "number ${count}" world.txt
> > + done &&
>
> Style: write "do" on its own line, aligned with "for" on the
> previous line. Same for "if/then/else/fi" "while/do/done" etc.
Done.
> An easy way to remember is that you never use any semicolon in
> your shell script, except for the double-semicolon you use to
> delimit your case arms.
>
> When any of these steps fail, you do not notice it, unless the
> failure happens to occur to "git commit" during the last round. I
> think you can use "return 1" to signal the failure to the test
> framework, like so.
>
> for count in $(test_seq 1 10)
> do
> echo hello >>world.txt &&
> git commit -m "number $count} world.txt ||
> return 1
> done &&
Done.
> ls -1 .git/reftable >files &&
> test_line_count = 2 files
>
Done.
> > + git reflog refs/heads/master > output &&
> > + grep "commit (initial): first post" output &&
> > + grep "commit: number 10" output
>
> Do we give guarantees that we keep all the reflog entries? Would it
> help to count the number of lines in the output file here?
The reflog entries are only removed if someone calls into the ref
backend to expire them. Does that happen automatically?
--
Han-Wen Nienhuys - Google Munich
I work 80%. Don't expect answers from me on Fridays.
--
Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
