diff mbox series

[02/21] tests: Add bash lib

Message ID	20240520090611.10406-3-jonatan.schlag@ipfire.org
State	New
Headers	From: Jonatan Schlag <jonatan.schlag@ipfire.org> To: development@lists.ipfire.org Subject: [PATCH 02/21] tests: Add bash lib Date: Mon, 20 May 2024 11:05:52 +0200 Message-Id: <20240520090611.10406-3-jonatan.schlag@ipfire.org> In-Reply-To: <20240520090611.10406-1-jonatan.schlag@ipfire.org> References: <20240520090611.10406-1-jonatan.schlag@ipfire.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Message-ID-Hash: VCHFVUPTKILNUOORJBCEO5MJ5GMDD7MU Precedence: list Archived-At: <https://lists.ipfire.org/hyperkitty/list/development@lists.ipfire.org/message/VCHFVUPTKILNUOORJBCEO5MJ5GMDD7MU/>
Series	[01/21] test: Add bash lib for colors \| [01/21] test: Add bash lib for colors [02/21] tests: Add bash lib [03/21] tests/lib.sh: Add function test_that_key_in_arry_has_value [04/21] tests/lib.sh: Add check if variable exists to test_that_key_in_arry_has_value [05/21] tests/lib.sh: Add logging functions [06/21] tests/lib.sh: adjust to pytest logging style [07/21] tests/lib.sh: Fix check if array is defined [08/21] test_that_key_in_arry_has_value: Check if the key is defined [09/21] tests/lib.sh: Use namref to access an array [10/21] test_that_key_in_arry_has_value: Check if a key in an array exists [11/21] tests: Add function to test the ouput of a bash function [12/21] initscript functions: add readhash [13/21] initscript fkt: ignore blank lines in readhash [14/21] test: Add functions test_that_array_is_defined [15/21] tests: Add functions test_that_array_doesnt_have_key [16/21] initscripts fkt: Ignore comments in readhash [17/21] initscripts fkt: ignore invalid keys in readhash [18/21] initscripts fkt: readhash should only parse lines with a = [19/21] initscripts fkt: Check for invalid values in readhash [20/21] initscripts fkt: keep readhash compatible with older implementation [21/21] initscripts fkt: Fix shebang

Commit Message

Jonatan Schlag May 20, 2024, 9:05 a.m. UTC

  This allows use to write test with less effort as we can reuse functions

Signed-off-by: Jonatan Schlag <jonatan.schlag@ipfire.org>
---
 tests/lib.sh | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 tests/lib.sh

Comments

Michael Tremer May 31, 2024, 9:47 a.m. UTC | #1

Hello,

> On 20 May 2024, at 10:05, Jonatan Schlag <jonatan.schlag@ipfire.org> wrote:
> 
> This allows use to write test with less effort as we can reuse functions
> 
> Signed-off-by: Jonatan Schlag <jonatan.schlag@ipfire.org>
> ---
> tests/lib.sh | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
> create mode 100644 tests/lib.sh
> 
> 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="$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")"

???

I don’t think this is very intuitive.

> +. ${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
> +}

We have had so many discussions about how to name functions. “That” is quite generic in my opinion.

Why not “test_command” or something like that?

> +
> +var_has_value() {
> + [[ "${!1}" == "${2}" ]]
> +}
> -- 
> 2.39.2
>

Jonatan Schlag June 2, 2024, 5:49 p.m. UTC | #2

Hi,


Am Freitag, dem 31.05.2024 um 10:47 +0100 schrieb Michael Tremer:
> Hello,
> 
> > On 20 May 2024, at 10:05, Jonatan Schlag
> > <jonatan.schlag@ipfire.org> wrote:
> > 
> > This allows use to write test with less effort as we can reuse
> > functions
> > 
> > Signed-off-by: Jonatan Schlag <jonatan.schlag@ipfire.org>
> > ---
> > tests/lib.sh | 20 ++++++++++++++++++++
> > 1 file changed, 20 insertions(+)
> > create mode 100644 tests/lib.sh
> > 
> > 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="$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")"
> 
> ???
> 
> I don’t think this is very intuitive.

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?

Jonatan

> 
> > +. ${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
> > +}
> 
> We have had so many discussions about how to name functions. “That”
> is quite generic in my opinion.
> 
> Why not “test_command” or something like that?
> 
> > +
> > +var_has_value() {
> > + [[ "${!1}" == "${2}" ]]
> > +}
> > -- 
> > 2.39.2
> > 
>

Michael Tremer June 3, 2024, 9:19 a.m. UTC | #3

Hello,

> On 2 Jun 2024, at 18:49, Jonatan Schlag <jonatan.schlag@ipfire.org> wrote:
> 
> 
> Hi,
> 
> 
> Am Freitag, dem 31.05.2024 um 10:47 +0100 schrieb Michael Tremer:
>> Hello,
>> 
>>> On 20 May 2024, at 10:05, Jonatan Schlag
>>> <jonatan.schlag@ipfire.org> wrote:
>>> 
>>> This allows use to write test with less effort as we can reuse
>>> functions
>>> 
>>> Signed-off-by: Jonatan Schlag <jonatan.schlag@ipfire.org>
>>> ---
>>> tests/lib.sh | 20 ++++++++++++++++++++
>>> 1 file changed, 20 insertions(+)
>>> create mode 100644 tests/lib.sh
>>> 
>>> 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="$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")"
>> 
>> ???
>> 
>> I don’t think this is very intuitive.
> 
> 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 also has some more advantages. For example, you can easily put a print statement 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 once. You need to add way more comments, please.

-Michael

> Jonatan
> 
>> 
>>> +. ${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
>>> +}
>> 
>> We have had so many discussions about how to name functions. “That”
>> is quite generic in my opinion.
>> 
>> Why not “test_command” or something like that?
>> 
>>> +
>>> +var_has_value() {
>>> + [[ "${!1}" == "${2}" ]]
>>> +}
>>> -- 
>>> 2.39.2

diff mbox series

Patch

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="$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")"
+
+. ${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
+}
+
+var_has_value() {
+	[[ "${!1}" == "${2}" ]]
+}