From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: Victoria Dye via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, derrickstolee@github.com, gitster@pobox.com,
Victoria Dye <vdye@github.com>, Victoria Dye <vdye@github.com>
Subject: Re: [PATCH 5/8] scalar-clone: add test coverage
Date: Thu, 1 Sep 2022 11:32:39 +0200 (CEST) [thread overview]
Message-ID: <o429p430-sp18-rr20-p2ro-4r60qs2n83o5@tzk.qr> (raw)
In-Reply-To: <a3b7cb0a3bd1f56172db8420b9e80a26be1fda5d.1661961746.git.gitgitgadget@gmail.com>
Hi Victoria,
On Wed, 31 Aug 2022, Victoria Dye via GitGitGadget wrote:
> From: Victoria Dye <vdye@github.com>
>
> Create a new test file ('t9211-scalar-clone.sh') to exercise the options and
> behavior of the 'scalar clone' command.
Great catch!
I have one suggestion, given my experience with debugging test failures:
> +test_expect_success 'creates content in enlistment root' '
> + test_when_finished cleanup_clone cloned &&
> +
> + scalar clone "file://$(pwd)/to-clone" cloned &&
This pattern of cloning into `cloned` and removing the directory when the
test case is done is repeated throughout this test script.
In instances where all test cases succeed, that poses no problem.
When running the test script with `-i`, also no problem.
But when we run into test failures in CI, those directories will be
removed before the workflow run can tar them up and upload them for later
inspection.
May I suggest an alternative strategy?
If we drop all those `test_when_finished cleanup_clone cloned` calls and
instead `scalar clone` into different directories (whose names reflect the
test cases' intended purpose), I could imagine having a much easier time
not only diagnosing but also reproducing and fixing test failures in the
future.
When discussing code review practices [*1*], we did not come up with any
standard terminology to describe what I am offering here, and I am unsure
how to label this in a catchy way. I want to present this suggestion for
you to consider, and I would be delighted if you take it, at the same time
I will happily let it go should you decide against it.
Ciao,
Dscho
Footnote *1*:
https://colabti.org/irclogger/irclogger_log/git-devel?date=2022-08-29#l48
next prev parent reply other threads:[~2022-09-01 9:32 UTC|newest]
Thread overview: 70+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-31 16:02 [PATCH 0/8] scalar: integrate into core Git Victoria Dye via GitGitGadget
2022-08-31 16:02 ` [PATCH 1/8] scalar: fix command documentation section header Victoria Dye via GitGitGadget
2022-08-31 16:02 ` [PATCH 2/8] scalar: include in standard Git build & installation Victoria Dye via GitGitGadget
2022-09-01 9:11 ` Johannes Schindelin
2022-09-01 13:17 ` [PATCH 0/5] Makefile: split up $(test_bindir_programs) Ævar Arnfjörð Bjarmason
2022-09-01 13:17 ` [PATCH 1/5] Makefile: factor sed-powered '#!/bin/sh' munging into a variable Ævar Arnfjörð Bjarmason
2022-09-01 13:17 ` [PATCH 2/5] Makefile: define "TEST_{PROGRAM,OBJS}" variables earlier Ævar Arnfjörð Bjarmason
2022-09-01 13:17 ` [PATCH 3/5] Makefile: simplify $(test_bindir_programs) rule by splitting it up Ævar Arnfjörð Bjarmason
2022-09-01 13:17 ` [PATCH 4/5] Makefile: define bin-wrappers/% rules with a template Ævar Arnfjörð Bjarmason
2022-09-01 13:17 ` [PATCH 5/5] Makefile: fix "make clean && make bin-wrappers/$NAME" dependencies Ævar Arnfjörð Bjarmason
2022-09-01 15:02 ` [PATCH 0/5] Makefile: split up $(test_bindir_programs) Derrick Stolee
2022-09-02 12:38 ` Johannes Schindelin
2022-10-26 14:42 ` [PATCH v2 0/3] Makefile: fix issues with bin-wrappers/% rule Ævar Arnfjörð Bjarmason
2022-10-26 14:42 ` [PATCH v2 1/3] Makefile: factor sed-powered '#!/bin/sh' munging into a variable Ævar Arnfjörð Bjarmason
2022-10-26 17:51 ` Junio C Hamano
2022-10-26 14:42 ` [PATCH v2 2/3] Makefile: define "TEST_{PROGRAM,OBJS}" variables earlier Ævar Arnfjörð Bjarmason
2022-10-26 16:47 ` Junio C Hamano
2022-10-26 18:47 ` Ævar Arnfjörð Bjarmason
2022-10-26 19:13 ` Junio C Hamano
2022-10-26 14:42 ` [PATCH v2 3/3] Makefile: simplify $(test_bindir_programs) rule by splitting it up Ævar Arnfjörð Bjarmason
2022-10-26 18:48 ` Junio C Hamano
2022-10-26 19:14 ` Ævar Arnfjörð Bjarmason
2022-10-26 20:28 ` Junio C Hamano
2022-10-26 20:43 ` Ævar Arnfjörð Bjarmason
2022-10-28 0:57 ` [PATCH v2 0/3] Makefile: fix issues with bin-wrappers/% rule Jeff King
2022-10-28 3:03 ` Ævar Arnfjörð Bjarmason
2022-10-31 22:28 ` [PATCH v3 0/4] Makefile: untangle bin-wrappers/% rule complexity Ævar Arnfjörð Bjarmason
2022-10-31 22:28 ` [PATCH v3 1/4] Makefile: factor sed-powered '#!/bin/sh' munging into a variable Ævar Arnfjörð Bjarmason
2022-10-31 22:28 ` [PATCH v3 2/4] Makefile: define "TEST_{PROGRAM,OBJS}" variables earlier Ævar Arnfjörð Bjarmason
2022-10-31 22:28 ` [PATCH v3 3/4] Makefile: rename "test_bindir_programs" variable, pre-declare Ævar Arnfjörð Bjarmason
2022-10-31 22:28 ` [PATCH v3 4/4] Makefile: simplify $(test_bindir_programs) rule by splitting it up Ævar Arnfjörð Bjarmason
2022-10-31 23:54 ` [PATCH v3 0/4] Makefile: untangle bin-wrappers/% rule complexity Taylor Blau
2022-11-01 1:29 ` Ævar Arnfjörð Bjarmason
2022-08-31 16:02 ` [PATCH 3/8] git help: special-case `scalar` Johannes Schindelin via GitGitGadget
2022-08-31 16:02 ` [PATCH 4/8] scalar: implement the `help` subcommand Johannes Schindelin via GitGitGadget
2022-08-31 16:48 ` Ævar Arnfjörð Bjarmason
2022-09-01 16:08 ` Victoria Dye
2022-09-01 8:51 ` Johannes Schindelin
2022-09-01 9:17 ` Johannes Schindelin
2022-08-31 16:02 ` [PATCH 5/8] scalar-clone: add test coverage Victoria Dye via GitGitGadget
2022-09-01 9:32 ` Johannes Schindelin [this message]
2022-09-01 23:49 ` Victoria Dye
2022-09-02 9:07 ` Johannes Schindelin
2022-09-02 16:52 ` Junio C Hamano
2022-08-31 16:02 ` [PATCH 6/8] t/perf: add Scalar performance tests Victoria Dye via GitGitGadget
2022-09-01 9:39 ` Johannes Schindelin
2022-09-01 16:15 ` Victoria Dye
2022-09-01 16:21 ` Victoria Dye
2022-09-02 9:16 ` Johannes Schindelin
2022-09-01 16:43 ` Junio C Hamano
2022-08-31 16:02 ` [PATCH 7/8] t/perf: add 'GIT_PERF_USE_SCALAR' run option Victoria Dye via GitGitGadget
2022-09-01 9:43 ` Johannes Schindelin
2022-09-02 4:00 ` Victoria Dye
2022-09-02 9:17 ` Johannes Schindelin
2022-08-31 16:02 ` [PATCH 8/8] Documentation/technical: include Scalar technical doc Victoria Dye via GitGitGadget
2022-08-31 17:03 ` [PATCH 0/8] scalar: integrate into core Git Ævar Arnfjörð Bjarmason
2022-08-31 18:42 ` Victoria Dye
2022-09-01 9:56 ` Johannes Schindelin
2022-09-02 15:56 ` [PATCH v2 0/9] " Victoria Dye via GitGitGadget
2022-09-02 15:56 ` [PATCH v2 1/9] scalar: fix command documentation section header Victoria Dye via GitGitGadget
2022-09-02 15:56 ` [PATCH v2 2/9] scalar: include in standard Git build & installation Victoria Dye via GitGitGadget
2022-09-02 15:56 ` [PATCH v2 3/9] git help: special-case `scalar` Johannes Schindelin via GitGitGadget
2022-09-02 15:56 ` [PATCH v2 4/9] scalar: implement the `help` subcommand Johannes Schindelin via GitGitGadget
2022-09-02 15:56 ` [PATCH v2 5/9] scalar: add to 'git help -a' command list Victoria Dye via GitGitGadget
2022-09-02 15:56 ` [PATCH v2 6/9] scalar-clone: add test coverage Victoria Dye via GitGitGadget
2022-09-02 15:56 ` [PATCH v2 7/9] t/perf: add Scalar performance tests Victoria Dye via GitGitGadget
2022-09-02 15:56 ` [PATCH v2 8/9] t/perf: add 'GIT_PERF_USE_SCALAR' run option Victoria Dye via GitGitGadget
2022-09-02 15:56 ` [PATCH v2 9/9] Documentation/technical: include Scalar technical doc Victoria Dye via GitGitGadget
2022-09-05 10:36 ` [PATCH v2 0/9] scalar: integrate into core Git Johannes Schindelin
2022-09-08 20:54 ` Derrick Stolee
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=o429p430-sp18-rr20-p2ro-4r60qs2n83o5@tzk.qr \
--to=johannes.schindelin@gmx.de \
--cc=derrickstolee@github.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=gitster@pobox.com \
--cc=vdye@github.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).