Re: [PATCH 02/21] tests: Add bash lib

Re: [PATCH 02/21] tests: Add bash lib

[Jonatan Schlag]

[-- Attachment #1: Type: text/plain, Size: 1442 bytes --]


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(a)ipfire.org> wrote:
> > 
> > This allows use to write test with less effort as we can reuse
> > functions
> > 
> > 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
> > 
> > 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
> > 
> 

Re: [PATCH 02/21] tests: Add bash lib

[Michael Tremer]

[-- Attachment #1: Type: text/plain, Size: 2237 bytes --]

Hello,

> On 2 Jun 2024, at 18:49, Jonatan Schlag <jonatan.schlag(a)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(a)ipfire.org> wrote:
>>> 
>>> This allows use to write test with less effort as we can reuse
>>> functions
>>> 
>>> 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
>>> 
>>> 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

[PATCH 02/21] tests: Add bash lib

[Jonatan Schlag]

[-- Attachment #1: Type: text/plain, Size: 751 bytes --]

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

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

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}" ]]
+}
-- 
2.39.2