[prev in list] [next in list] [prev in thread] [next in thread] 

List:       git
Subject:    Re: [PATCH] test-lib-functions: drop redundant diagnostic print
From:       Ævar Arnfjörð Bjarmason <avarab () gmail ! com>
Date:       2022-10-31 20:16:07
Message-ID: 221031.864jvj7noz.gmgdl () evledraar ! gmail ! com
[Download RAW message or body]


On Mon, Oct 31 2022, Martin =C3=85gren wrote:

> `test_path_is_missing` was introduced back in 2caf20c52b ("test-lib:
> user-friendly alternatives to test [-d|-f|-e]", 2010-08-10). It took the
> path that was supposed to be missing, as well as an optional "diagnosis"
> that would be echoed if the path was found to be alive.
>
> Commit 45a2686441 ("test-lib-functions: remove bug-inducing
> "diagnostics" helper param", 2021-02-12) dropped this diagnostic
> functionality from several `test_path_is_foo` helpers, but note how it
> tweaked the README entry on `test_path_is_missing` without actually
> adjusting its implementation.
>
> Commit e7884b353b ("test-lib-functions: assert correct parameter count",
> 2021-02-12) then followed up by asserting that we get just a single
> argument.
>
> This history leaves us in a state where we assert that we have exactly
> one argument, then go on to anyway check for arguments, echoing them
> all. It's clear that we can simplify this code. We should also note that
> we run `ls -ld "$1"`, so printing the filename a second time doesn't
> really buy us anything. Thus, we can drop the whole `if` block as
> redundant.
>
> Signed-off-by: Martin =C3=85gren <martin.agren@gmail.com>
> ---
>  t/test-lib-functions.sh | 4 ----
>  1 file changed, 4 deletions(-)
>
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index 29d914a12b..796093a7b3 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -921,10 +921,6 @@ test_path_is_missing () {
>  	then
>  		echo "Path exists:"
>  		ls -ld "$1"
> -		if test $# -ge 1
> -		then
> -			echo "$*"
> -		fi
>  		false
>  	fi
>  }

Thanks, this is an obviously good change, sorry about leaving that loose
end.
[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic