[PATCH 0/1] bg_setenv: added basic tests for bg_setenv CLI

[Michael Adler]

Hi,

I'd like to introduce some basic tests for bg_setenv/bg_printenv.

I know that there are unit-tests (and typically I prefer them to tests like these) but nevertheless I am convinced that
these kind of tests are useful: they can catch errors which unit-tests simply cannot do (e.g. linking, see our
discussion here [1].

The original motivation for these tests was to test the new bg_printenv CLI options which I have implemented (but not
yet submitted) and to ensure that I did not break the existing interface.

Following the "upstream first" principle, I'm hereby trying to bring them upstream.

Kind regards,
Michael

[1] https://www.mail-archive.com/efibootg...@googlegroups.com/msg01351.html

Michael Adler (1):
bg_setenv: added basic tests for bg_setenv CLI

.github/workflows/main.yaml | 5 +-
.gitmodules | 9 +++
docs/COMPILE.md | 3 +-
tests/bats | 1 +
tests/bg_setenv.bats | 108 +++++++++++++++++++++++++++++++++
tests/files/BGENV.DAT | Bin 0 -> 132104 bytes
tests/test_helper/bats-assert | 1 +
tests/test_helper/bats-support | 1 +
8 files changed, 126 insertions(+), 2 deletions(-)
create mode 160000 tests/bats
create mode 100755 tests/bg_setenv.bats
create mode 100644 tests/files/BGENV.DAT
create mode 160000 tests/test_helper/bats-assert
create mode 160000 tests/test_helper/bats-support

--
2.33.0

[Michael Adler]

The bg_setenv/bg_printenv command-line interface is currently untested,
which makes it error-prone to extend the current code.

This commit introduces basic tests for the bg_setenv/bg_printenv
command-line interface.

The following new test dependencies are introduced (included as git
submodules, as suggested in the docs [1]):

- bats-core [2]: MIT
- bats-support [3]: CC0-1.0
- bats-assert [4]: CC0-1.0

Strictly speaking, only bats-core is a must-have, but [3] and [4] make
it much more friendly to use (and they are being used in this commit as
well).

[1] https://bats-core.readthedocs.io/en/stable/tutorial.html#quick-installation
[2] https://github.com/bats-core/bats-core
[3] https://github.com/bats-core/bats-support
[4] https://github.com/bats-core/bats-assert

Signed-off-by: Michael Adler <michae...@siemens.com>
---

.github/workflows/main.yaml | 5 +-
.gitmodules | 9 +++
docs/COMPILE.md | 3 +-
tests/bats | 1 +
tests/bg_setenv.bats | 108 +++++++++++++++++++++++++++++++++
tests/files/BGENV.DAT | Bin 0 -> 132104 bytes
tests/test_helper/bats-assert | 1 +
tests/test_helper/bats-support | 1 +
8 files changed, 126 insertions(+), 2 deletions(-)
create mode 160000 tests/bats
create mode 100755 tests/bg_setenv.bats
create mode 100644 tests/files/BGENV.DAT
create mode 160000 tests/test_helper/bats-assert
create mode 160000 tests/test_helper/bats-support

diff --git a/.github/workflows/main.yaml b/.github/workflows/main.yaml
index 6bb9f66..e283cb2 100644
--- a/.github/workflows/main.yaml
+++ b/.github/workflows/main.yaml
@@ -86,9 +86,12 @@ jobs:
- name: Build amd64
if: ${{ matrix.target == 'amd64' }}
run: |
- cd build
+ pushd build >/dev/null
../configure
make check -j $(nproc)
+ sudo make install
+ popd >/dev/null
+ time ./tests/bats/bin/bats --tap --print-output-on-failure tests
- name: Build i386
if: ${{ matrix.target == 'i386' }}
run: |
diff --git a/.gitmodules b/.gitmodules
index 80ad11c..941b2fd 100644
--- a/.gitmodules
+++ b/.gitmodules
@@ -1,3 +1,12 @@
[submodule "tests/fff"]
path = tests/fff
url = https://github.com/meekrosoft/fff
+[submodule "tests/bats"]
+ path = tests/bats
+ url = https://github.com/bats-core/bats-core.git
+[submodule "tests/test_helper/bats-support"]
+ path = tests/test_helper/bats-support
+ url = https://github.com/bats-core/bats-support.git
+[submodule "tests/test_helper/bats-assert"]
+ path = tests/test_helper/bats-assert
+ url = https://github.com/bats-core/bats-assert.git
diff --git a/docs/COMPILE.md b/docs/COMPILE.md
index bda19d4..61650a2 100644
--- a/docs/COMPILE.md
+++ b/docs/COMPILE.md
@@ -50,4 +50,5 @@ where `<sys-root-dir>` points to the wanted sysroot for cross-compilation.

## Testing ##

-`make check` will run all unit tests.
+* `make check` will run all unit tests.
+* `./tests/bats/bin/bats tests` will run all integration tests (`bats` is a git submodule)
diff --git a/tests/bats b/tests/bats
new file mode 160000
index 0000000..01636e4
--- /dev/null
+++ b/tests/bats
@@ -0,0 +1 @@
+Subproject commit 01636e489bb9b9d6c430465e6f8409422e63d3fa
diff --git a/tests/bg_setenv.bats b/tests/bg_setenv.bats
new file mode 100755
index 0000000..a329f0e
--- /dev/null
+++ b/tests/bg_setenv.bats
@@ -0,0 +1,108 @@
+#!/usr/bin/env bats
+# Copyright (c) Siemens AG, 2021
+#
+# Authors:
+# Michael Adler <michae...@siemens.com>
+#
+# This work is licensed under the terms of the GNU GPL, version 2. See
+# the COPYING file in the top-level directory.
+#
+# SPDX-License-Identifier: GPL-2.0
+#
+
+setup() {
+ load 'test_helper/bats-support/load'
+ load 'test_helper/bats-assert/load'
+
+ # get the containing directory of this file
+ # use $BATS_TEST_FILENAME instead of ${BASH_SOURCE[0]} or $0,
+ # as those will point to the bats executable's location or the preprocessed file respectively
+ DIR="$( cd "$( dirname "$BATS_TEST_FILENAME" )" >/dev/null 2>&1 && pwd )"
+ PATH="$DIR/..:$PATH"
+}
+
+@test "print existing BGENV" {
+ envfile="$DIR/files/BGENV.DAT"
+
+ run bg_printenv -f "$envfile"
+ assert_output "Values:
+in_progress: no
+revision: 1
+kernel: C:BOOT:kernel.efi
+kernelargs: root=/dev/sda
+watchdog timeout: 0 seconds
+ustate: 0 (OK)
+
+user variables:
+foo = bar"
+
+ run md5sum "$envfile"
+ assert_output --regexp "6613ca4da6bcaad37362f4096a60f0e3\s*$envfile"
+}
+
+@test "create an empty BGENV" {
+ envfile="$BATS_TEST_TMPDIR/BGENV.DAT"
+
+ run bg_setenv -f "$envfile"
+
+ assert_output "Output written to $envfile."
+
+ run stat --format=%s "$envfile"
+ assert_output 132104
+
+ run bg_printenv -f "$envfile"
+ assert_output "Values:
+in_progress: no
+revision: 0
+kernel:
+kernelargs:
+watchdog timeout: 0 seconds
+ustate: 0 (OK)
+
+user variables:"
+ run md5sum "$envfile"
+ assert_output --regexp "441b49e907a117d2fe1dc1d69d8ea1b0\s*$envfile"
+}
+
+@test "modify BGENV, discard existing values" {
+ envfile="$BATS_TEST_TMPDIR/BGENV.DAT"
+
+ cp "$DIR/files/BGENV.DAT" "$envfile"
+ run bg_setenv -f "$envfile" -k C:BOOTNEW:kernel.efi
+
+ run bg_printenv -f "$envfile"
+ assert_output "Values:
+in_progress: no
+revision: 0
+kernel: C:BOOTNEW:kernel.efi
+kernelargs:
+watchdog timeout: 0 seconds
+ustate: 0 (OK)
+
+user variables:"
+
+ run md5sum "$envfile"
+ assert_output --regexp "15bc40c9feae99cc879cfc55e0132caa\s*$envfile"
+}
+
+@test "modify BGENV, preserve existing values" {
+ envfile="$BATS_TEST_TMPDIR/BGENV.DAT"
+
+ cp "$DIR/files/BGENV.DAT" "$envfile"
+ run bg_setenv -f "$envfile" -k C:BOOTNEW:kernel.efi -P
+
+ run bg_printenv -f "$envfile"
+ assert_output "Values:
+in_progress: no
+revision: 1
+kernel: C:BOOTNEW:kernel.efi
+kernelargs: root=/dev/sda
+watchdog timeout: 0 seconds
+ustate: 0 (OK)
+
+user variables:
+foo = bar"
+
+ run md5sum "$envfile"
+ assert_output --regexp "56f5733996e25fa1ea9b053d8f197658\s*$envfile"
+}
diff --git a/tests/files/BGENV.DAT b/tests/files/BGENV.DAT
new file mode 100644
index 0000000000000000000000000000000000000000..1dbbfe51dbbeddd43c2ed228d713a47d43e5dda5
GIT binary patch
literal 132104
zcmeIw!3{xC5CzaH8c>1?h{Q@_Ljg7#_<}@;FP~^ZgKDuh3srpQO)_)0bLOqH$SRwv
zZKZP#>2j@p$)x%*r+EDp=+ag_vdlF5y7#l@y;4&vU+Oo^a?8BG>)R}YGB~zvMm5Jj
zk6qLKl0kp~0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+
z009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBly
zK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF
z5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk
z1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs
z0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZ
zfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&U
zAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C7
z2oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N
z0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+
z009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBly
NK!Cu%34C6|_ZORb5l;XB

literal 0
HcmV?d00001

diff --git a/tests/test_helper/bats-assert b/tests/test_helper/bats-assert
new file mode 160000
index 0000000..672ad18
--- /dev/null
+++ b/tests/test_helper/bats-assert
@@ -0,0 +1 @@
+Subproject commit 672ad1823a4d2f0c475fdbec0c4497498eec5f41
diff --git a/tests/test_helper/bats-support b/tests/test_helper/bats-support
new file mode 160000
index 0000000..d140a65
--- /dev/null
+++ b/tests/test_helper/bats-support
@@ -0,0 +1 @@
+Subproject commit d140a65044b2d6810381935ae7f0c94c7023c8c3
--
2.33.0

[Jan Kiszka]

On 20.10.21 09:05, Michael Adler wrote:
> The bg_setenv/bg_printenv command-line interface is currently untested,
> which makes it error-prone to extend the current code.
>
> This commit introduces basic tests for the bg_setenv/bg_printenv
> command-line interface.
>
> The following new test dependencies are introduced (included as git
> submodules, as suggested in the docs [1]):
>
> - bats-core [2]: MIT
> - bats-support [3]: CC0-1.0
> - bats-assert [4]: CC0-1.0
>
> Strictly speaking, only bats-core is a must-have, but [3] and [4] make
> it much more friendly to use (and they are being used in this commit as
> well).
>

Your argumentation why to introduce another test framework and why one
can't replace the other should go here as well. To some degree it was in
the cover letter, but here it would be easier to find later on via git.

BUT: You do not explain to us anywhere why installing bats from a
distro, Debian or Ubuntu in CI, is not sufficient. Why do we need to
import it?

Jan

--
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux

[Michael Adler]

> Your argumentation why to introduce another test framework and why one
> can't replace the other should go here as well. To some degree it was in
> the cover letter, but here it would be easier to find later on via git.

Sure, I will extend the commit description in V2.

> BUT: You do not explain to us anywhere why installing bats from a
> distro, Debian or Ubuntu in CI, is not sufficient. Why do we need to
> import it?

Older versions of Debian and Ubuntu ship the discontinued version [1], while newer versions have switched to "the" fork
[2]. However, we would still be missing the following packages:

> > - bats-support [3]: CC0-1.0
> > - bats-assert [4]: CC0-1.0

They are available on Arch Linux, but not on Ubuntu/Debian.

I could adjust the tests and get rid of [3] and [4] (I guess we wouldn't miss out on that much), thus allowing us to get
rid of [1] as well.

Personally, I don't mind either way. We already depend on a git submodule for testing purposes, which is why I didn't
hesitate to introduce a few more. Pandora's box had already been opened ;)

Let me know what you think and I'll proceed accordingly.

Kind regards,
Michael

[1] https://github.com/sstephenson/bats
[2] https://github.com/bats-core/bats-core

--
Michael Adler

Siemens AG
T RDA IOT SES-DE
Otto-Hahn-Ring 6
81739 München, Deutschland

Siemens Aktiengesellschaft: Vorsitzender des Aufsichtsrats: Jim Hagemann Snabe; Vorstand: Roland Busch, Vorsitzender; Klaus Helmrich, Cedrik Neike, Matthias Rebellius, Ralf P. Thomas, Judith Wiese; Sitz der Gesellschaft: Berlin und München, Deutschland; Registergericht: Berlin-Charlottenburg, HRB 12300, München, HRB 6684; WEEE-Reg.-Nr. DE 23691322

[Michael Adler]

This commit introduces basic tests for the bg_setenv/bg_printenv
command-line interface.

Rationale: The bg_setenv/bg_printenv command-line interface is currently

untested, which makes it error-prone to extend the current code.

Even though there are existing unit tests (based on fff [1]), they are
unable to catch certain kinds of issues, e.g. linking related, see our
discussion here [2]. For that reason, a new test dependency is
introduced: bats-core (MIT, [3]).

[1] https://github.com/meekrosoft/fff
[2] https://www.mail-archive.com/efibootg...@googlegroups.com/msg01351.html
[3] https://github.com/bats-core/bats-core

Signed-off-by: Michael Adler <michae...@siemens.com>
---

.github/workflows/main.yaml | 7 ++-
docs/COMPILE.md | 3 +-
tests/bg_setenv.bats | 105 ++++++++++++++++++++++++++++++++++++

tests/files/BGENV.DAT | Bin 0 -> 132104 bytes

4 files changed, 112 insertions(+), 3 deletions(-)

create mode 100755 tests/bg_setenv.bats
create mode 100644 tests/files/BGENV.DAT

diff --git a/.github/workflows/main.yaml b/.github/workflows/main.yaml
index 6bb9f66..10b2a2d 100644
--- a/.github/workflows/main.yaml
+++ b/.github/workflows/main.yaml
@@ -40,7 +40,7 @@ jobs:
if: ${{ matrix.target == 'amd64' || matrix.target == 'cppcheck' }}
run: |
sudo apt-get install --no-install-recommends \
- gcc-multilib gnu-efi libz-dev libpci-dev check
+ gcc-multilib gnu-efi libz-dev libpci-dev check bats
- name: Install i386 dependencies

if: ${{ matrix.target == 'i386' }}
run: |

@@ -86,9 +86,12 @@ jobs:
- name: Build amd64
if: ${{ matrix.target == 'amd64' }}
run: |
- cd build
+ pushd build >/dev/null
../configure
make check -j $(nproc)
+ sudo make install
+ popd >/dev/null

+ time bats --tap tests

- name: Build i386
if: ${{ matrix.target == 'i386' }}
run: |

diff --git a/docs/COMPILE.md b/docs/COMPILE.md
index bda19d4..df52c07 100644

--- a/docs/COMPILE.md
+++ b/docs/COMPILE.md
@@ -50,4 +50,5 @@ where `<sys-root-dir>` points to the wanted sysroot for cross-compilation.

## Testing ##

-`make check` will run all unit tests.
+* `make check` will run all unit tests.

+* `bats tests` will run all integration tests (`bats` is a git submodule)

diff --git a/tests/bg_setenv.bats b/tests/bg_setenv.bats
new file mode 100755

index 0000000..bebd914
--- /dev/null
+++ b/tests/bg_setenv.bats
@@ -0,0 +1,105 @@

+#!/usr/bin/env bats
+# Copyright (c) Siemens AG, 2021
+#
+# Authors:
+# Michael Adler <michae...@siemens.com>
+#
+# This work is licensed under the terms of the GNU GPL, version 2. See
+# the COPYING file in the top-level directory.
+#
+# SPDX-License-Identifier: GPL-2.0
+#
+
+setup() {

+ # get the containing directory of this file
+ # use $BATS_TEST_FILENAME instead of ${BASH_SOURCE[0]} or $0,
+ # as those will point to the bats executable's location or the preprocessed file respectively
+ DIR="$( cd "$( dirname "$BATS_TEST_FILENAME" )" >/dev/null 2>&1 && pwd )"
+ PATH="$DIR/..:$PATH"
+}
+
+@test "print existing BGENV" {
+ envfile="$DIR/files/BGENV.DAT"
+
+ run bg_printenv -f "$envfile"

+ [[ "$output" = "Values:

+in_progress: no
+revision: 1
+kernel: C:BOOT:kernel.efi
+kernelargs: root=/dev/sda
+watchdog timeout: 0 seconds
+ustate: 0 (OK)
+
+user variables:
+foo = bar" ]]
+
+ run md5sum "$envfile"

+ [[ "$output" =~ ^6613ca4da6bcaad37362f4096a60f0e3\s*.* ]]

+}
+
+@test "create an empty BGENV" {

+ envfile="$(mktemp -d)/BGENV.DAT"

+
+ run bg_setenv -f "$envfile"
+

+ [[ "$output" = "Output written to $envfile." ]]

+
+ run stat --format=%s "$envfile"

+ [[ "$output" = 132104 ]]

+
+ run bg_printenv -f "$envfile"

+ [[ "$output" = "Values:

+in_progress: no
+revision: 0
+kernel:
+kernelargs:
+watchdog timeout: 0 seconds
+ustate: 0 (OK)
+
+user variables:" ]]
+ run md5sum "$envfile"

+ [[ "$output" =~ ^441b49e907a117d2fe1dc1d69d8ea1b0\s*.* ]]

+}
+
+@test "modify BGENV, discard existing values" {

+ envfile="$(mktemp -d)/BGENV.DAT"

+
+ cp "$DIR/files/BGENV.DAT" "$envfile"
+ run bg_setenv -f "$envfile" -k C:BOOTNEW:kernel.efi
+
+ run bg_printenv -f "$envfile"

+ [[ "$output" = "Values:

+in_progress: no
+revision: 0
+kernel: C:BOOTNEW:kernel.efi
+kernelargs:
+watchdog timeout: 0 seconds
+ustate: 0 (OK)
+
+user variables:" ]]
+
+ run md5sum "$envfile"

+ [[ "$output" =~ ^15bc40c9feae99cc879cfc55e0132caa\s*.* ]]

+}
+
+@test "modify BGENV, preserve existing values" {

+ envfile="$(mktemp -d)/BGENV.DAT"

+
+ cp "$DIR/files/BGENV.DAT" "$envfile"
+ run bg_setenv -f "$envfile" -k C:BOOTNEW:kernel.efi -P
+
+ run bg_printenv -f "$envfile"

+ [[ "$output" = "Values:

+in_progress: no
+revision: 1
+kernel: C:BOOTNEW:kernel.efi
+kernelargs: root=/dev/sda
+watchdog timeout: 0 seconds
+ustate: 0 (OK)
+
+user variables:
+foo = bar" ]]
+
+ run md5sum "$envfile"

+ [[ "$output" =~ ^56f5733996e25fa1ea9b053d8f197658\s*.* ]]

+}
diff --git a/tests/files/BGENV.DAT b/tests/files/BGENV.DAT
new file mode 100644
index 0000000000000000000000000000000000000000..1dbbfe51dbbeddd43c2ed228d713a47d43e5dda5
GIT binary patch
literal 132104
zcmeIw!3{xC5CzaH8c>1?h{Q@_Ljg7#_<}@;FP~^ZgKDuh3srpQO)_)0bLOqH$SRwv
zZKZP#>2j@p$)x%*r+EDp=+ag_vdlF5y7#l@y;4&vU+Oo^a?8BG>)R}YGB~zvMm5Jj
zk6qLKl0kp~0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+
z009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBly
zK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF
z5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk
z1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs
z0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZ
zfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&U
zAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C7
z2oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N
z0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+
z009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBly
NK!Cu%34C6|_ZORb5l;XB

literal 0
HcmV?d00001

--
2.33.0

[Michael Adler]

> Sure, I will extend the commit description in V2.

[x] Done (see patch V2).

> I could adjust the tests and get rid of [3] and [4] (I guess we wouldn't miss out on that much), thus allowing us to get
> rid of [1] as well.

[x] Done (see patch V2).

Discussion may continue in V2 thread.

Kind regards,
Michael

[Jan Kiszka]

How was this created? Can't we do that via a command before the test?

Jan

> literal 132104
> zcmeIw!3{xC5CzaH8c>1?h{Q@_Ljg7#_<}@;FP~^ZgKDuh3srpQO)_)0bLOqH$SRwv
> zZKZP#>2j@p$)x%*r+EDp=+ag_vdlF5y7#l@y;4&vU+Oo^a?8BG>)R}YGB~zvMm5Jj
> zk6qLKl0kp~0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+
> z009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBly
> zK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF
> z5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk
> z1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs
> z0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZ
> zfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&U
> zAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C7
> z2oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N
> z0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+
> z009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBly
> NK!Cu%34C6|_ZORb5l;XB
>
> literal 0
> HcmV?d00001
>

--

[Michael Adler]

> How was this created? Can't we do that via a command before the test?

My motivation to store BGENV.DAT in git was to ensure backwards compatibility for bg_printenv. However, that can also be
achieved by computing the checksum of the generated file, thus there is actually no need to store the binary file in the
repository. See patch v3.

--
2.33.0

[Michael Adler]

This commit introduces basic tests for the bg_setenv/bg_printenv
command-line interface.

Rationale: The bg_setenv/bg_printenv command-line interface is currently
untested, which makes it error-prone to extend the current code.

Even though there are existing unit tests (based on fff [1]), they are
unable to catch certain kinds of issues, e.g. linking related, see our
discussion here [2]. For that reason, a new test dependency is
introduced: bats-core (MIT, [3]).

[1] https://github.com/meekrosoft/fff
[2] https://www.mail-archive.com/efibootg...@googlegroups.com/msg01351.html
[3] https://github.com/bats-core/bats-core

Signed-off-by: Michael Adler <michae...@siemens.com>
---
.github/workflows/main.yaml | 7 ++-
docs/COMPILE.md | 3 +-

tests/bg_setenv.bats | 115 ++++++++++++++++++++++++++++++++++++
3 files changed, 122 insertions(+), 3 deletions(-)
create mode 100755 tests/bg_setenv.bats

index bda19d4..0d6c09c 100644

--- a/docs/COMPILE.md
+++ b/docs/COMPILE.md
@@ -50,4 +50,5 @@ where `<sys-root-dir>` points to the wanted sysroot for cross-compilation.

## Testing ##

-`make check` will run all unit tests.
+* `make check` will run all unit tests.

+* `bats tests` will run all integration tests (you need [bats-core](https://github.com/bats-core/bats-core) for that; packaged as `bash-bats` for Arch Linux and `bats` for Debian).

diff --git a/tests/bg_setenv.bats b/tests/bg_setenv.bats
new file mode 100755

index 0000000..dffcb5f
--- /dev/null
+++ b/tests/bg_setenv.bats
@@ -0,0 +1,115 @@

+#!/usr/bin/env bats
+# Copyright (c) Siemens AG, 2021
+#
+# Authors:
+# Michael Adler <michae...@siemens.com>
+#
+# This work is licensed under the terms of the GNU GPL, version 2. See
+# the COPYING file in the top-level directory.
+#
+# SPDX-License-Identifier: GPL-2.0
+#
+
+setup() {
+ # get the containing directory of this file
+ # use $BATS_TEST_FILENAME instead of ${BASH_SOURCE[0]} or $0,
+ # as those will point to the bats executable's location or the preprocessed file respectively
+ DIR="$( cd "$( dirname "$BATS_TEST_FILENAME" )" >/dev/null 2>&1 && pwd )"
+ PATH="$DIR/..:$PATH"
+}
+

+create_sample_bgenv() {
+ bg_setenv -f "$1" \
+ --kernel=C:BOOT:kernel.efi \
+ --args=root=/dev/sda \
+ --uservar=foo=bar \
+ --revision=1
+}
+
+@test "ensure BGENV.DAT backwards compatbility" {
+ local envfile
+ envfile="$(mktemp -d)/BGENV.DAT"
+ create_sample_bgenv "$envfile"

+
+ run bg_printenv -f "$envfile"
+ [[ "$output" = "Values:
+in_progress: no
+revision: 1
+kernel: C:BOOT:kernel.efi
+kernelargs: root=/dev/sda
+watchdog timeout: 0 seconds
+ustate: 0 (OK)
+
+user variables:
+foo = bar" ]]
+
+ run md5sum "$envfile"

+ [[ "$output" =~ ^6ad1dd1d98209a03d7b4fc2d2f16f9ec\s*.* ]]
+}
+
+@test "create an empty BGENV.DAT" {
+ local envfile

+ envfile="$(mktemp -d)/BGENV.DAT"
+
+ run bg_setenv -f "$envfile"
+ [[ "$output" = "Output written to $envfile." ]]
+

+ run md5sum "$envfile"
+ [[ "$output" =~ ^441b49e907a117d2fe1dc1d69d8ea1b0\s*.* ]]
+
+ run bg_printenv -f "$envfile"
+ [[ "$output" = "Values:
+in_progress: no
+revision: 0
+kernel:
+kernelargs:
+watchdog timeout: 0 seconds
+ustate: 0 (OK)
+
+user variables:" ]]
+}
+
+@test "modify BGENV, discard existing values" {

+ local envfile

+ envfile="$(mktemp -d)/BGENV.DAT"
+

+ create_sample_bgenv "$envfile"

+ run bg_setenv -f "$envfile" -k C:BOOTNEW:kernel.efi
+
+ run bg_printenv -f "$envfile"
+ [[ "$output" = "Values:
+in_progress: no
+revision: 0
+kernel: C:BOOTNEW:kernel.efi
+kernelargs:
+watchdog timeout: 0 seconds
+ustate: 0 (OK)
+
+user variables:" ]]
+
+ run md5sum "$envfile"
+ [[ "$output" =~ ^15bc40c9feae99cc879cfc55e0132caa\s*.* ]]
+}
+
+@test "modify BGENV, preserve existing values" {

+ local envfile

+ envfile="$(mktemp -d)/BGENV.DAT"
+

+ create_sample_bgenv "$envfile"

+ run bg_setenv -f "$envfile" -k C:BOOTNEW:kernel.efi -P
+
+ run bg_printenv -f "$envfile"
+ [[ "$output" = "Values:
+in_progress: no
+revision: 1
+kernel: C:BOOTNEW:kernel.efi
+kernelargs: root=/dev/sda
+watchdog timeout: 0 seconds
+ustate: 0 (OK)
+
+user variables:
+foo = bar" ]]
+
+ run md5sum "$envfile"

+ [[ "$output" =~ ^a24b154a48e1f33b79b87e0fa5eff8a1\s*.* ]]
+}
--
2.33.0

[Jan Kiszka]

Thanks, applied, just wrapping over-long lines.

Jan