Re: [PATCH v2 2/4] t1517: test commands that are designed to be run outside repository

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

 



On Mon, May 13, 2024 at 12:21 PM Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> A few commands, like "git apply" and "git patch-id", have been
> broken with a recent change to stop setting the default hash
> algorithm to SHA-1.  Test them and fix them in later commits.
>
> Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
> ---
>  t/t1517-outside-repo.sh | 61 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 61 insertions(+)
>  create mode 100755 t/t1517-outside-repo.sh
>
> diff --git a/t/t1517-outside-repo.sh b/t/t1517-outside-repo.sh
> new file mode 100755
> index 0000000000..e0fd495ec1
> --- /dev/null
> +++ b/t/t1517-outside-repo.sh
> @@ -0,0 +1,61 @@
> +#!/bin/sh
> +
> +test_description='check random commands outside repo'
> +
> +TEST_PASSES_SANITIZE_LEAK=true
> +. ./test-lib.sh
> +
> +test_expect_success 'set up a non-repo directory and test file' '
> +       GIT_CEILING_DIRECTORIES=$(pwd) &&
> +       export GIT_CEILING_DIRECTORIES &&
> +       mkdir non-repo &&
> +       (
> +               cd non-repo &&
> +               # confirm that git does not find a repo
> +               test_must_fail git rev-parse --git-dir
> +       ) &&
> +       test_write_lines one two three four >nums &&
> +       git add nums &&
> +       cp nums nums.old &&
> +       test_write_lines five >>nums &&
> +       git diff >sample.patch
> +'
> +
> +test_expect_failure 'compute a patch-id outside repository' '

Do we only expect failure because of a temporary condition (the bug
that is mentioned in the commit message)? If so, we should probably
add a TODO, FIXME, or some other similar style of comment that
describes that this should be fixed. This way, if the patch series to
fix the issue doesn't materialize, people don't read the test file and
think that these commands aren't supported outside of a repository.

Do we have a way of catching the specific failure mode? i.e. if it
crashes, is there a test_expect_crash? I'm thinking that it might be
nice to be more specific about what kind of failure we expect, this
way if it fails in a different way before we convert these to
test_expect_success, the test fails (due to the unexpected change).
I'm assuming that for crashes of this type there's no good/portable
way of verifying that it's a specific crash, but having the test check
for a difference between an exit code that indicates a signal was
raised and an exit code that indicates that the process returned
"naturally" after an unsuccessful execution might be feasible, if we
already have such a mechanism. Adding one just for this series doesn't
seem justified.

> +       git patch-id <sample.patch >patch-id.expect &&
> +       (
> +               cd non-repo &&
> +               git patch-id <../sample.patch >../patch-id.actual
> +       ) &&
> +       test_cmp patch-id.expect patch-id.actual
> +'
> +
> +test_expect_failure 'hash-object outside repository' '
> +       git hash-object --stdin <sample.patch >hash.expect &&
> +       (
> +               cd non-repo &&
> +               git hash-object --stdin <../sample.patch >../hash.actual
> +       ) &&
> +       test_cmp hash.expect hash.actual
> +'
> +
> +test_expect_failure 'apply a patch outside repository' '
> +       (
> +               cd non-repo &&
> +               cp ../nums.old nums &&
> +               git apply ../sample.patch
> +       ) &&
> +       test_cmp nums non-repo/nums
> +'
> +
> +test_expect_success 'grep outside repository' '
> +       git grep --cached two >expect &&
> +       (
> +               cd non-repo &&
> +               cp ../nums.old nums &&
> +               git grep --no-index two >../actual
> +       ) &&
> +       test_cmp expect actual
> +'
> +
> +test_done
> --
> 2.45.0-145-g3e4a232f6e
>
>





[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