Re: [PATCH v2 1/9] t5526: introduce test helper to assert on fetches
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Feb 15, 2022, 21:37 UTC
- Message-ID
- <220215.86v8xfx134.gmgdl@evledraar.gmail.com>
- In-Reply-To
- <20220215172318.73533-2-chooglen@google.com>
On Wed, Feb 16 2022, Glen Choo wrote:
Show 14 quoted lines
> +verify_fetch_result() {
> + ACTUAL_ERR=$1 &&
> + rm -f expect.err.combined &&
> + if [ -f expect.err.super ]; then
> + cat expect.err.super >>expect.err.combined
> + fi &&
> + if [ -f expect.err.sub ]; then
> + cat expect.err.sub >>expect.err.combined
> + fi &&
> + if [ -f expect.err.deep ]; then
> + cat expect.err.deep >>expect.err.combined
> + fi &&
> + test_cmp expect.err.combined $ACTUAL_ERR
> +}I see this will get further modified in this series, but I wondered why not just something like:
diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh index 5d55f14ed42..4d8e06dea52 100755 --- a/t/t5526-fetch-submodules.sh +++ b/t/t5526-fetch-submodules.sh @@ -49,16 +49,12 @@ add_upstream_commit() { # expect.err file should be rm-ed. verify_fetch_result() { ACTUAL_ERR=$1 && - rm -f expect.err.combined && - if [ -f expect.err.super ]; then - cat expect.err.super >>expect.err.combined - fi && - if [ -f expect.err.sub ]; then - cat expect.err.sub >>expect.err.combined - fi && - if [ -f expect.err.deep ]; then - cat expect.err.deep >>expect.err.combined - fi && + + { + cat expect.err.super + cat expect.err.sub + cat expect.err.deep + } >expect.err.combined test_cmp expect.err.combined $ACTUAL_ERR } I.e. there's no law that we've got to avoid non-zero on every exit code. In this case we can avoid the existence checks and just "cat" them together, and if it fails we'll presumably fail on the test_cmp anyway...