Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automated testing script #215

Merged
merged 5 commits into from
Jul 9, 2024

Conversation

Lockszmith-GH
Copy link
Contributor

@Lockszmith-GH Lockszmith-GH commented Jul 1, 2024

I created this script to test my changes with #210.

  • automated tests related changes I've made to jlmkr: added argument that can be passed to log and status so that interactive features of joutnalctl could be avoided.

How the test-jlmkr bash script works is detailed in the next comment.

@Lockszmith-GH Lockszmith-GH changed the base branch from main to develop July 1, 2024 23:23
@Lockszmith-GH
Copy link
Contributor Author

Lockszmith-GH commented Jul 1, 2024

Feel free to add your suggestions.

The script has 2 optional parameter invocation sets:

  • <jail type> [<jail name>]
  • <template/path> <jail name>

If the script is invoked without arguments, it will use the default configuration and name the test jail default-jail.

Legend:

Arg name Description
<jail type> The template dir-name in $JLMKR_PATH/templates (it will load the config file within it)
<jail name>* The name of the jail that will be created and destroyed during the testing.
If not supplied, the default is <jail type>-test
<template/path> relative or absolute path to a config template. (<jail name> must be supplied)

PRE-REQUISITE:
* WARNING: If <jail name> exists, it will be removed.

Environment variables control the test behavior:

Variable name Default Description
JLMKR_PATH optional - see description When unspecified, and SCALE_POOL_ROOT is defined, it will check if $SCALE_POOL_ROOT/jailmaker contians the jlmkr.py script. Otherwise, it will use ${PWD:-.} instead (the local dir)
SCALE_POOL_ROOT optional Used to define JLMKR_PATH, it should point to the root of the ZFS dataset hosting the jailmaker script dir.
STOP 0 0 - perform all non-blocking tests
l - only list and images, nothing else
i - interactive test, includ console-blocking waiting for input tests (edit and shell)
FULL_TEST 0 0 - perform a single run
1* - perform a full-test

* FULL_TEST=1 will perform a full run ONLY when passing a <jail type> (will not work with <template/path>)

Type of Run

In STOP=0 (the default) all steps, with the interactive steps manipulated to run without prompting.
The interactive steps are steps which prompt the user for input. These are shell and edit.
In STOP=i all steps will be performed, and interactive steps will wait for user input.

A special shorthand option exists STOP=l which will just run list and images, this is to test basic invocation of the script regardless of specific jails. This is also the only non-destructive mode.

Single Run

By default, a single run in non-interactive mode will run, it runs from whatever CWD path the shell is in.

Full Test

The Full test, starts with the single run (whether interactive or not), if successful, it continues to perform non-interactive tests with the following parameters:

S F CWD Path to template
+ + $JLMKR_PATH ./templates/$JAIL_TYPE/config
+ JLMKR_PATH templates/$JAIL_TYPE/config
+ JLMKR_PATH $SCALE_POOL_ROOT/jailmaker/$JAIL_CONFIG
+ ~ JLMKR_PATH/$JAIL_CONFIG
+ ~ a temporary file, it's contents will be copied form the <jail type>'s config file.

S - Single Run | F - Full Test

Example invocation:

Single run, interactive mode, nixos jail type:

cd ${SCALE_POOL_ROOT:?must define this before running}/jailmaker \
&& sudo STOP=i SCALE_POOL_ROOT=$SCALE_POOL_ROOT $SCALE_POOL_ROOT/jailmaker/test/test-jlmkr nixos

Full test, default settings, nixos jail type:

cd ${JLMKR_PATH:?must define this before running} \
&& sudo FULL_TEST=1 ./test/test-jlmkr nixos

What to expect

A full non-interactive test run with nixos (which is rather lean, yet tests both pre hook and init scripts) ran for ~260 seconds.

The report summary outputs what type of run it was with CWD and path to config file and for each step, a green checkmark indicating ✅ Success, a red X symbol following the exit code❌(<error code>) for erros. Both follow the command executed.
In the case a step wasn't performed, a blank checkbox 🔳 followed by the name of the step will be listed.
The report is always listed in alphabetic order (although the steps are performed in an order that allows testing as much as possible while taking into account dependent steps.

@Jip-Hop
Copy link
Owner

Jip-Hop commented Jul 3, 2024

Thanks as always!

I gave the test script a quick spin and I like it :)

STOP=i - interactive test, includ console-blocking waiting for input tests (edit and shell)

I think we can and should make the script fully non-interactive. Testing shell can be done similar to exec. See this example:

./jlmkr.py shell nixos /bin/sh -c 'ifconfig'

And edit could be tested this way EDITOR=cat ./jlmkr.py edit test.

Would you mind taking a look at my initial attempt at an automatic test script? This runs on each commit and PR (you can unfold Run a one-line script to see the logs). Your script covers a lot more cases so it would be great to run that.

I had to override the systemd_nspawn_user_args with --network-veth to make it work though because the VM which runs the job doesn't have a br0 interface...

@Jip-Hop
Copy link
Owner

Jip-Hop commented Jul 3, 2024

P.S. the develop branch already contains the code to pass args to log and status so this PR doesn't need to modify jlmkr.py and would currently introduce conflicts if I were to merge it as is. Please remove any modifications to jlmkr.py in your automated_testing branch for now.

@Lockszmith-GH
Copy link
Contributor Author

NP, I'll rebase on develop, and kick out any changes that might conflict.
I'll take a look - probably later this week.

@Lockszmith-GH
Copy link
Contributor Author

Cleaned up the branch, it now only included the shell script.

@Lockszmith-GH
Copy link
Contributor Author

Would you mind taking a look at ...

That's aweseom, and yes I agree - testing these test-cases would be helpful. Still need to think about how to properly test commits to new configs.

Next, I'll add your notes about shell and and edit - good ideas. - But feel free to merge if you don't want to wait

@Lockszmith-GH
Copy link
Contributor Author

Add non-interactive testing of shell and edit to the default.
Fixed some shellcheck issues as well

@Lockszmith-GH
Copy link
Contributor Author

I think you can now use the stdin fix #208 with the test script I add to make sure any config you're testing you're replacing the networking configuration (with sed or some other tool) to match the environment you're running on (like GitHub actions in the case of #86)

@Jip-Hop
Copy link
Owner

Jip-Hop commented Jul 5, 2024

Yeah that or preprocess the config files with sed from the GitHub actions test script. If you like you can experiment with this since pushing new commits in this PR will trigger the automatic testing action: https://github.com/Jip-Hop/jailmaker/pull/215/checks

@Lockszmith-GH
Copy link
Contributor Author

@Jip-Hop I some changes to the script so it would run properly form test.yml - let me know if you want any changes made ot the script.

NOTE - that it's currently just running the 'Single Run' test, which seems to be the one you'll want to run frequently.

The script has 2 optional parameter invocation sets:
* `<jail type>` [`<jail name>`]
* `<template/path>` `<jail name>`

Legend:
| Arg name | Description |
|-|-|
| `<jail type>` | The template dir-name in `$SCALE_POOL_ROOT/jailmaker/templates` (it will load the `config` file within it) |
| `<jail name>`\* | The name of the jail that will be created and destroyed during the testing. <br/> If not supplied, the default is `<jail type>-test` |
| `<template/path>` | relative or absolute path to a config template. (`<jail name>` must be supplied) |

> PRE-REQUISITE:
> \* WARNING: If `<jail name>` exists, it will be removed.

Environment variables control the test behavior:
| Variable name | Default |Description |
|-|:-:|-|
|`SCALE_POOL_ROOT`|N/A, must be exported|Env var must be defined and valid pointing to the root of the ZFS dataset hosting the `jailmaker` script dir. |
|`STOP`|0| `0` - perform all non-blocking tests<br/>`l` - only list and images, nothing else<br/>`i` - interactive test, includ console-blocking waiting for input tests (edit and shell)|
|`FULL_TEST`|0|`0` - perform a single run<br/>`1`* - perform a full-test|

> \* `FULL_TEST=1` will perform a full run ONLY when passing a `<jail type>` (will not work with `<template/path>`)

-- Type of Run

In `STOP=0` (the default) all steps, except the interactive steps will be performed.
The interactive steps are steps which prompt the user for input. These are `shell` and `edit`.
In `STOP=i` all steps will be performed.

A special shorthand option exists `STOP-l` which will just run `list` and `images`, this is to test basic invocation of the script regardless of specific jails. This is also the only non-destructive mode.

-- Single Run

By default, a single run in non-interactive mode will run, it runs from whatever CWD path the shell is in.

-- Full Test

The Full test, starts with the single run (whether interactive or not), if successful, it continues to perform non-interactive tests with the following parameters:

| S | F | CWD | Path to template |
|:-:|:-:|-|-|
|+|+|`$SCALE_POOL_ROOT/jailmaker`|`./templates/$JAIL_TYPE/config`|
||+|`$SCALE_POOL_ROOT/jailmaker`|`templates/$JAIL_TYPE/config`|
||+|`$SCALE_POOL_ROOT/jailmaker`|`$SCALE_POOL_ROOT/jailmaker/$JAIL_CONFIG`|
||+| ~ |`$SCALE_POOL_ROOT/jailmaker/$JAIL_CONFIG`|
||+| ~ | a _temporary file_, it's contents will be copied form the `<jail type>`'s config file. |

> S - Single Run | F - Full Test

--- Example invocation:
Single run, interactive mode, `nixos` jail type:

```bash
cd ${SCALE_POOL_ROOT:?must define this before running}/jailmaker \
&& sudo STOP=i SCALE_POOL_ROOT=$SCALE_POOL_ROOT $SCALE_POOL_ROOT/jailmaker/test/test-jlmkr nixos
```

Full test, default settings, `nixos` jail type:

```bash
cd ${SCALE_POOL_ROOT:?must define this before running}/jailmaker \
&& sudo FULL_TEST=1 SCALE_POOL_ROOT=$SCALE_POOL_ROOT $SCALE_POOL_ROOT/jailmaker/test/test-jlmkr nixos
```

--- What to expect
A full non-interactive test run with `nixos` (which is rather lean, yet tests both pre hook and init scripts) ran for ~280 seconds.

The report summary outputs what type of run it was with CWD and path to config file and for each step, a green checkmark indicating ✅ Success, a red X symbol following the exit code❌(`<error code>`) for erros. Both follow the command executed.
In the case a step wasn't performed, a blank checkbox 🔳 followed by the name of the step will be listed.
The report is always listed in alphabetic order (although the steps are performed in an order that allows testing as much as possible while taking into account dependent steps.
- fixed: if a failure happened, report will not print
- added: remove will always be attempted, even if failure would occurr
Modifying test-jlmkr to allow running from current dir
@Jip-Hop
Copy link
Owner

Jip-Hop commented Jul 9, 2024

Looks good and it's quick! Could you add a docker and/or podman test too? They are currently failing for reasons unknown to me: #220 (comment)

I don't think the additional docker/podman test needs to go through the whole suite of restart, status etc. And I think we should keep in mind that we can't always trust the exit code. So testing for 'side effects' (e.g. check if a file is actually created, or check if string is present in output with grep) could help increase confidence in test results.

@Lockszmith-GH
Copy link
Contributor Author

I've updated the instructions in the comment above.

@Lockszmith-GH
Copy link
Contributor Author

So testing for 'side effects' (e.g. check if a file is actually created, or check if string is present in output with grep) could help increase confidence in test results.

I agree, but I would suggest we start using it as is, and modify this later.
I believe a change in design might be needed to support that (maybe even drop the shell script and replace it with something more robust - maybe I'll even try python - though not my comfort zone)

Would you be ok opening a set of issues to tackle each requirement?
I feel it will allow more focused discussions.

@Jip-Hop Jip-Hop merged commit 434e195 into Jip-Hop:develop Jul 9, 2024
1 check passed
@Lockszmith-GH Lockszmith-GH deleted the automated_testing branch July 9, 2024 03:02
@Jip-Hop
Copy link
Owner

Jip-Hop commented Jul 9, 2024

Merged! For now it runs both test scripts.

I've updated the instructions in the comment above.

I put it in a readme. See develop branch.

maybe I'll even try python

Even better, haha!

Would you be ok opening a set of issues to tackle each requirement?

Please do! :)

P.S. another bug slipped by manual testing :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants