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

tests: Add pytest and nushell based tests #625

Merged
merged 3 commits into from
Jun 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .dockerignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
.cosa
target
!target/dev-rootfs
# These directories don't contribute to our container build
docs/
plans/
8 changes: 8 additions & 0 deletions .github/workflows/python.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
name: Python static analysis
on: [push, pull_request]
jobs:
ruff:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: chartboost/ruff-action@v1
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ test-tmt:
validate:
cargo fmt
cargo clippy
ruff check
.PHONY: validate

vendor:
Expand Down
22 changes: 20 additions & 2 deletions hack/provision-derived.sh
Original file line number Diff line number Diff line change
@@ -1,9 +1,26 @@
#!/bin/bash
set -xeu
case "$1" in
variant=$1
# I'm a big fan of nushell for interactive use, and I want to support
# using it in our test suite because it's better than bash. First,
# enable EPEL to get it.
. /usr/lib/os-release
if echo $ID_LIKE $ID | grep -q centos; then
dnf config-manager --set-enabled crb
dnf -y install epel-release epel-next-release
fi
# Ensure this is pre-created
mkdir -p -m 0700 /var/roothome
mkdir -p ~/.config/nushell
echo '$env.config = { show_banner: false, }' > ~/.config/nushell/config.nu
touch ~/.config/nushell/env.nu
dnf -y install nu
# And we also add pytest, to support running tests written in Python
dnf -y install python3-pytest
case "$variant" in
tmt)
# tmt wants rsync
dnf -y install cloud-init rsync && dnf clean all
dnf -y install cloud-init rsync
ln -s ../cloud-init.target /usr/lib/systemd/system/default.target.wants
# And tmt wants to write to /usr/local/bin
rm /usr/local -rf && ln -sr /var/usrlocal /usr/local && mkdir -p /var/usrlocal/bin
Expand All @@ -14,3 +31,4 @@ case "$1" in
echo "Unknown variant: $1" exit 1
;;
esac
dnf clean all && rm /var/log/* -rf
18 changes: 12 additions & 6 deletions plans/integration-run.fmf
Original file line number Diff line number Diff line change
@@ -1,12 +1,18 @@
# This tmt test just demonstrates local tmt usage.
# We'll hopefully expand it to do more interesting things in the
# future and unify with the other test plans.
# Run this via `make test-tmt` which will build a container,
# and a disk image from it.
provision:
how: virtual
# Generated by `cargo xtask `
# Generated by make test-tmt
image: file://./target/testvm/disk.qcow2
disk: 20
summary: Basic smoke test
summary: Execute booted tests
execute:
how: tmt
script: bootc status
# There's currently two dynamic test frameworks; python and nushell.
# python is well known and understood. nushell is less well known, but
# is quite nice for running subprocesses and the like while making
# it easy to parse JSON etc.
script: |
set -xeu
pytest tests/booted/*.py
ls tests/booted/*-test-*.nu |sort -n | while read t; do nu $t; done
1 change: 1 addition & 0 deletions tests/booted/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
__pycache__
8 changes: 8 additions & 0 deletions tests/booted/001-test-status.nu
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
use std assert
use tap.nu

tap begin "verify bootc status --json looks sane"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I read in the commit message that dynamic languages are not the preferred choice but I do wonder if maybe pytest might deserve a second look? Combined with tooling like pylint/mypy/ruff it many(or only some?) of the dynamic language pain points go away and while subprocess management is a little bit more complicated it's also more explicit, i.e. it's always clear what an arg for execv() is (i.e. less quoting issues). The upside(s) would be that it's more familiar to many people and that many of the things a testsuite needs (asserts, diffing if assert fails, error reporting etc) are already available.

This particular example in pytest wouldn't look too bad (IMHO):

import json
import subprocess


def test_bootc_status_json_smoke() -> None:
    """ verify bootc status --json looks sane """
    st = json.loads(subprocess.check_output(["bootc", "status", "--json"]))
    assert  st["apiVersion"] == "org.containers.bootc/v1alpha1"

Of course this might change once a more complex or pipeline heavy example come into play. But of course those are only my 2¢ and I can see that nushell has many upsides too!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, that example is trivial; here's a less trivial one that motivated this that's already in another PR:

https://github.com/containers/bootc/pull/620/files#diff-e6320bbd16d432b6d223da82f11037166a0da7a0c81b2575c4505aeaf6085cc9

More generally...what I've been trying to aim for is recreating some of the flavor/experience I'm used to with the "coreos-assembler kola external tests", which today are mostly shell script (and by which I have often been burned), but I like the framework around it.

Now admittedly...that test ended up being only about half running external binaries; less than I thought. I am totally not a nushell expert and so don't take it as any kind of reference example, but it felt somewhat nice. Especially the second half where it's really mostly just forking external binaries.

(A tangential aside, if you want a good sense of nushell, I came across https://github.com/nushell/nushell/blob/main/.github/workflows/release-pkg.nu which is very much the kind of thing that might start out as a small bash script split out of a Github actions run invocation which is 98% forking things; then later it grows from 10 to 100+ lines and you feel the need for better language features, but turning it into Python again I think would make it a good bit more unnecessarily verbose than it is nushell)

This all said...the next day after sending up this PR I definitely was feeling conflicted about introducing a new (relatively) obscure language (though I was pleased to discover it has a language server at least!). Introducing new languages into a codebase isn't something to do lightly - it implies a large ongoing maintenance and forces context switching for everyone who wants to do the basics of change the code and change the tests in this case.

Combined with tooling like pylint/mypy/ruff it many(or only some?) of the dynamic language pain points go away

I admit I stopped writing Python day to day years ago now, and the language has changed a lot. I came across
https://kobzol.github.io/rust/python/2023/05/20/writing-python-like-its-rust.html recently which was really useful.

and while subprocess management is a little bit more complicated it's also more explicit, i.e. it's always clear what an arg for execv() is (i.e. less quoting issues).

To be clear, nushell and also the xshell macro we're already using in the Rust-based tests are both concise and don't have that quoting issue. (bash of course, does unless you rigorously add quotes, or you add shellcheck to your CI, which 90% of the time points out unnecessary things)


Anyways bigger picture...the other strong arguments for Python here is that:

  • It's already in the base images we are targeting
  • It's a language people already either know, or can learn easily
  • In today's world LLMs know Python well too and they can be very helpful for translating/explaining/creating
  • It's already used for tests in bib

And of these things the latter is interesting...getting to a point where bootc and bib actually share more logic around building and testing seems like an important goal. So given that, I'm fine adding tests in Python too...let me try updating this PR to do that.

(But yes the downside is I personally have low experience with the Python ecosystem like pytest etc., but I can learn)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On a related topic, I did reference in the commit message:

The podman project has a mix of a "bats" suite which is all bash based

Which if you look at the surface like https://github.com/containers/podman/blob/7b4f6ec576ca6419db8d9a3d0a93c635ae05fccc/test/system/055-rm.bats is...generally OK I think. But if you look at the implementation, things like https://github.com/containers/podman/blob/7b4f6ec576ca6419db8d9a3d0a93c635ae05fccc/test/system/helpers.bash#L908 are an eldritch horror...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This language stuff is easy to get sidetracked on, but just one other interesting bit...nushell is less dynamic than either bash or python; by default each script is parsed and type-checked before being executed at all. For example, just interactively in my shell:

> ls | from json
Error: nu::parser::input_type_mismatch

  × Command does not support table input.
   ╭─[entry #1:1:6]
 1 │ ls | from json
   ·      ────┬────
   ·          ╰── command doesn't support table input
   ╰────

And typoing a variable name in a nushell script is caught by default. Of course though it's still a pretty dynamic language, a lot of things have the "any" type and there's lots of ways to fail at runtime.

Anyways...testing some python and pytest now.


let st = bootc status --json | from json
assert equal $st.apiVersion org.containers.bootc/v1alpha1
tap ok
4 changes: 4 additions & 0 deletions tests/booted/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# Booted tests

These are intended to run via tmt; use e.g.
`make test-tmt`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might want to add a note that this requires a rootful connection to podman, e.g. export CONTAINER_CONNECTION="podman-machine-default-root". Also might be good to note this will appear to hang while copying the bootc source into the container since the source directory is large.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also might be good to note this will appear to hang while copying the bootc source into the container since the source directory is large.

See teemtee/tmt#3037

12 changes: 12 additions & 0 deletions tests/booted/basic.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# Tests which are read-only/nondestructive

import json
import subprocess

def run(*args):
subprocess.check_call(*args)

def test_bootc_status():
o = subprocess.check_output(["bootc", "status", "--json"])
st = json.loads(o)
assert st['apiVersion'] == 'org.containers.bootc/v1alpha1'
15 changes: 15 additions & 0 deletions tests/booted/tap.nu
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# A simple nushell "library" for the
# "Test anything protocol":
# https://testanything.org/tap-version-14-specification.html
export def begin [description] {
print "TAP version 14"
print $description
}

export def ok [] {
print "ok"
}

export def fail [] {
print "not ok"
}
2 changes: 2 additions & 0 deletions xtask/src/xtask.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,8 @@ fn man2markdown(sh: &Shell) -> Result<()> {
#[context("test-integration")]
fn test_tmt(sh: &Shell) -> Result<()> {
cmd!(sh, "cargo run -p tests-integration run-vm prepare-tmt").run()?;
// cc https://pagure.io/testcloud/pull-request/174
cmd!(sh, "rm -vf /var/tmp/tmt/testcloud/images/disk.qcow2").run()?;
cmd!(sh, "tmt run plans -n integration-run").run()?;
Ok(())
}
Expand Down
Loading