From mboxrd@z Thu Jan  1 00:00:00 1970
From: Michael Tremer <michael.tremer@ipfire.org>
To: development@lists.ipfire.org
Subject: Re: [PATCH 02/21] tests: Add bash lib
Date: Mon, 03 Jun 2024 10:19:18 +0100
Message-ID: <343A215D-4700-46E9-987B-D93AFE969664@ipfire.org>
In-Reply-To: <075647e956d0f2f58f72fce0448ce5b1a6369aa3.camel@ipfire.org>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============0306965839204314687=="
List-Id: <development.lists.ipfire.org>

--===============0306965839204314687==
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

Hello,

> On 2 Jun 2024, at 18:49, Jonatan Schlag <jonatan.schlag(a)ipfire.org> wrote:
>=20
>=20
> Hi,
>=20
>=20
> Am Freitag, dem 31.05.2024 um 10:47 +0100 schrieb Michael Tremer:
>> Hello,
>>=20
>>> On 20 May 2024, at 10:05, Jonatan Schlag
>>> <jonatan.schlag(a)ipfire.org> wrote:
>>>=20
>>> This allows use to write test with less effort as we can reuse
>>> functions
>>>=20
>>> Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
>>> ---
>>> tests/lib.sh | 20 ++++++++++++++++++++
>>> 1 file changed, 20 insertions(+)
>>> create mode 100644 tests/lib.sh
>>>=20
>>> diff --git a/tests/lib.sh b/tests/lib.sh
>>> new file mode 100644
>>> index 000000000..7749d5158
>>> --- /dev/null
>>> +++ b/tests/lib.sh
>>> @@ -0,0 +1,20 @@
>>> +#!/usr/bin/bash
>>> +
>>> +LIB_DIR=3D"$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")"
>>=20
>> ???
>>=20
>> I don=E2=80=99t think this is very intuitive.
>=20
> Ok, what am I supposed to do about it? I need the path where this file
> is located, and I cannot hard-code it. Should I add a comment?

Generally speaking, I like to split things into one line per task. So that it=
 is very easy to see what is happening when reading the code. This approach a=
lso has some more advantages. For example, you can easily put a print stateme=
nt between lines to see if you are getting the output that you are expecting.=
 That makes debugging nice and easy.

When I say generally, there are of course exceptions like very trivial things=
 that can often be put into the same line.

Commenting is also a lot easier when you only have to comment one thing at on=
ce. You need to add way more comments, please.

-Michael

> Jonatan
>=20
>>=20
>>> +. ${LIB_DIR}/lib_color.sh
>>> +
>>> +test_that() {
>>> +
>>> + if ! "$@" ; then
>>> + echo -e "${CLR_RED_BG} Test failed: ${*} ${CLR_RESET}"
>>> + return 1
>>> + else
>>> + echo -e "${CLR_GREEN_BG} Test succeded: ${*} ${CLR_RESET}"
>>> + return 0
>>> + fi
>>> +}
>>=20
>> We have had so many discussions about how to name functions. =E2=80=9CThat=
=E2=80=9D
>> is quite generic in my opinion.
>>=20
>> Why not =E2=80=9Ctest_command=E2=80=9D or something like that?
>>=20
>>> +
>>> +var_has_value() {
>>> + [[ "${!1}" =3D=3D "${2}" ]]
>>> +}
>>> --=20
>>> 2.39.2



--===============0306965839204314687==--