feat(cli): implement test subcommand for autopkgtests - #26
Conversation
TheJJ
left a comment
There was a problem hiding this comment.
wooh! well done :)
some ideas for improvements:
- why is test part of build/ and test_intent is not? maybe have a test/ subdirectory for both.
- i think we should factor out a containerconfig, and the container drivers to a container/ module outside of build, they are used by build and test (and soon lint, maybe more). the buildconfig and testconfig can then use the containerconfig.
74c1df5 to
546d340
Compare
546d340 to
e065763
Compare
Cleanly separates out environment and driver specific functionality and configs which can be reused in both build and test modules.
This is already done by the environment drivers. The only exception is the bare driver where we assume the host environment to provide an up-to-date apt update
e065763 to
35e242e
Compare
TheJJ
left a comment
There was a problem hiding this comment.
great restructure and generalization of the drivers! just a few nits and ideas left, but would be ready to ship now.
| | `docker` | Container (`isolation-container`) | | ||
| | `bare` | None — tests run as root on the host; requires `--allow-host-test` | | ||
|
|
||
| The driver *is* the testbed, so autopkgtest is told to run tests whose isolation restrictions the environment actually satisfies (`--ignore-restrictions`, only for those rungs). Tests that declare `Restrictions: isolation-container` therefore run on Docker/LXD/Incus instead of skipping. `isolation-machine` is not provided by any current driver (none is a VM); those tests still skip. Bare provides nothing, even with `--allow-host-test`. |
There was a problem hiding this comment.
lxd can be a vm - for convenience the best thing would be that the test env is upgraded to be a vm when a test requires isolation machine
There was a problem hiding this comment.
I think we always should have our driver selection and behavior deterministic and not automagic.
I.e. if we try running a test which requires vm isolation hint that you can use the lxd driver.
But that one would also need virtualization available on the host which might not necessarily be possible out of the box so automagicness is probably not that desirable here
There was a problem hiding this comment.
yea, and the lxd should get a --vm flag or something
| ); | ||
| } else { | ||
| eprintln!("Build failed: {error}"); | ||
| eprintln!("Re-run with --shell-on-failure to inspect the build environment"); |
There was a problem hiding this comment.
or if its persistent, use debmagic shell? (which could if not given a "target" prompt interacively for the build/test env (whichever failed last by default)?
There was a problem hiding this comment.
Hmm, I think the debmagic shell command now needs a rework anyways, I think we need to remove that as an explicit command and make it subcommands of the debmagic build / test commands. Otherwise it might be too confusing
| Ok(()) | ||
| } | ||
|
|
||
| pub fn resource_name(prefix: &str, label: &str, identifier: &str) -> String { |
There was a problem hiding this comment.
max len could be a parameter, since only lxd has this length limit i think
There was a problem hiding this comment.
I think having it restricted in general does not hurt much and keeps variance in our code low
No description provided.