feat(cli): implement test subcommand for autopkgtests - #26
Conversation
TheJJ
left a comment
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.
|
|
||
| ## TL;DR | ||
|
|
||
| - Entry point: `debmagic test` — runs tests from `debian/tests/control` against the `.debs` of a prior `debmagic build` |
There was a problem hiding this comment.
.debs is not right, just leave it out and say need prior build
| | `--source-dir <dir>` | Directory containing the `debian/` package directory | | ||
| | `--allow-host-test` | Allow the bare driver, which runs autopkgtest as root on the host | | ||
|
|
||
| [`debmagic shell`](#inspecting-a-failed-test-run) — attach an interactive shell to a test environment |
There was a problem hiding this comment.
how do we select build/test environment?
| | `1` | Test failure, testbed error, or other autopkgtest error | | ||
| | `2` | Strict-only failure: skipped tests or no tests declared under `--strict` | | ||
|
|
||
| autopkgtest skips tests whose `Restrictions:` the `null` backend cannot satisfy (e.g. `isolation-container`, `isolation-machine`). Skips are reported loudly; use `--strict` to escalate them to exit code 2. |
There was a problem hiding this comment.
we can pass capabilities to the null driver which it then pretends to have. and we are in a container (or even lxd vm), so we can give those caps to the null driver.
| .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidInput, e))?; | ||
|
|
||
| self.exec_in_container(cmd, Some(&container_path), requires_root, env_add) | ||
| self.exec_in_container_exit_status(cmd, Some(&container_path), requires_root, env_add) |
There was a problem hiding this comment.
naming it exit_status is weird. why not just exec_in_container
| } | ||
|
|
||
| fn run_command_env( | ||
| fn run_command_exit_status( |
There was a problem hiding this comment.
why exit_status, it's not that special, run_command alone would be a better name
| let base_config = prior_build | ||
| .as_ref() | ||
| .map(|metadata| metadata.config.clone()) | ||
| .unwrap_or_else(|| BuildConfig { |
There was a problem hiding this comment.
this seems wrong here, what do we need to construct a whole BuildConfig for?
| purpose: EnvironmentPurpose::Test, | ||
| }); | ||
|
|
||
| let test_config = BuildConfig { |
There was a problem hiding this comment.
all signing etc is unneeded for testing. this looks like we need a separate containerconfig, which is then used by buildconfig and testconfig
|
|
||
| let apt_env = [("DEBIAN_FRONTEND", "noninteractive")]; | ||
| test_run.driver.run_command_env( | ||
| &["apt-get", "update"], |
There was a problem hiding this comment.
this is not something that should be involed here. if needed, a "initial package list update" flag should be passed to the container driver.
| changes_filename, | ||
| &format!("{source_tree_name}/"), | ||
| "--", | ||
| "null", |
There was a problem hiding this comment.
here we can pass more capabilities to null, e.g. isolation container/vm (if environment is vm)
| let outcome = map_autopkgtest_exit(exit_code, intent.strict); | ||
|
|
||
| if outcome == TestOutcome::Failed && stdout().is_terminal() { | ||
| eprintln!("Tests failed (autopkgtest exit code {exit_code}). Dropping into shell..."); |
There was a problem hiding this comment.
always a shell is probably annoying, if one iterates on a fix.
2 participants
No description provided.