Re: [PATCH 5/8] scalar-clone: add test coverage

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Victoria,

On Wed, 31 Aug 2022, Victoria Dye via GitGitGadget wrote:

> From: Victoria Dye <vdye@xxxxxxxxxx>
>
> 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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux