Conversation
Add config-driven pytest test suites with typed TestSuiteConfig. Test
suites are defined in [test-suites] with type = "pytest" and a [pytest]
subtable specifying working-dir, test-paths (glob-expanded), and
extra-args (with {image-path} placeholder substitution).
Rewrite the 'image test' command to resolve test suites from project
config, dispatch to the pytest runner, and support --test-suite filtering,
--junit-xml output, and automatic image artifact resolution.
Local LISA-specific test runner code is temporarily replaced; we will
bring it back with the new TOML-driven configuration for local test
execution.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds config-driven pytest test suite support to azldev image test, including new typed test-suite configuration, schema updates, and a native pytest runner that handles glob-expanded test paths and placeholder substitution.
Changes:
- Extend project config with
TestSuiteConfigtype = "pytest"and a[pytest]subtable (plus schema + validation + merge/path resolution updates). - Rewrite
azldev image testto resolve suites from project config, support--test-suitefiltering, auto-resolve image artifacts, and optionally emit JUnit XML. - Introduce a native pytest runner that creates/reuses a venv, installs dependencies, expands globs, and runs
python -m pytest.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| schemas/azldev.schema.json | Adds schema definitions for pytest test suite config. |
| scenario/snapshots/TestSnapshots_config_generate-schema_stdout_1.snap | Updates schema generation snapshot. |
| scenario/snapshots/TestSnapshotsContainer_config_generate-schema_stdout_1.snap | Updates container schema generation snapshot. |
| internal/projectconfig/testsuite.go | Introduces typed test suite config, pytest config, validation, merge, and absolute-path resolution. |
| internal/projectconfig/testsuite_test.go | Adds unit tests for new test suite validation/merge behavior. |
| internal/projectconfig/configfile.go | Validates test suite configs during config file validation. |
| internal/projectconfig/loader.go | Merges test suites and resolves their relative paths to absolute paths. |
| internal/projectconfig/loader_test.go | Updates/adds loader tests for pytest suite parsing and validation failures. |
| internal/projectconfig/project.go | Minor refactor/rename in test suite reference validation. |
| internal/app/azldev/cmds/image/test.go | Reworks image test command to use config-defined suites and new flags. |
| internal/app/azldev/cmds/image/test_test.go | Updates command wiring/flag tests for the new interface. |
| internal/app/azldev/cmds/image/pytestrunner.go | Adds native pytest runner: venv creation, dependency install, arg building, glob expansion. |
| internal/app/azldev/cmds/image/pytestrunner_test.go | Adds unit tests for pytest arg building and placeholder/glob behavior. |
| internal/app/azldev/cmds/image/boot.go | Generalizes artifact discovery and adds OCI artifact format support; splits “bootable” vs “all” formats. |
| internal/app/azldev/cmds/image/boot_test.go | Updates tests for new format helpers and OCI extension inference. |
| docs/user/reference/cli/azldev_image_test.md | Auto-regenerated CLI docs reflecting the new image test UX and flags. |
| // Validate test suite configurations. | ||
| for suiteName, suite := range f.TestSuites { | ||
| suite.Name = suiteName | ||
|
|
||
| if err := suite.Validate(); err != nil { | ||
| return fmt.Errorf("invalid test suite %#q:\n%w", suiteName, err) | ||
| } | ||
| } |
There was a problem hiding this comment.
Test suite names (map keys) are not validated before being used elsewhere as path components (e.g., pytest venv dir under the work dir). A suite name containing path separators or traversal (like '..') could escape the intended directory. Consider validating suiteName with fileutils.ValidateFilename (or equivalent) here during config validation so unsafe names are rejected early with a clear error.
| cfg := env.Config() | ||
| if cfg == nil { | ||
| return errors.New("no project configuration loaded") | ||
| } |
There was a problem hiding this comment.
The error returned when no config is loaded ("no project configuration loaded") is pretty opaque for a user-facing CLI. Consider returning a more actionable message that indicates 'image test' requires a project config and suggests running from a project directory or passing '-C' / '--config-file'.
| func (t *TestSuiteConfig) Validate() error { | ||
| switch t.Type { | ||
| case TestTypePytest: | ||
| if t.Pytest == nil { | ||
| return fmt.Errorf("%w: test suite %#q of type %#q requires a [pytest] subtable", | ||
| ErrMissingTestField, t.Name, t.Type) | ||
| } | ||
|
|
||
| if err := t.Pytest.Validate(); err != nil { | ||
| return fmt.Errorf("test suite %#q: %w", t.Name, err) | ||
| } | ||
|
|
||
| // NOTE: When adding a new test type with its own subtable field (e.g., Lisa *LisaConfig), | ||
| // add a mismatch check here: | ||
| // if t.Lisa != nil { return fmt.Errorf("%w: ...", ErrMismatchedTestSubtable) } | ||
| // and add the symmetric check in the new type's case branch. | ||
|
|
||
| default: | ||
| return fmt.Errorf("%w: %#q (test suite: %#q)", ErrUnknownTestType, t.Type, t.Name) | ||
| } |
There was a problem hiding this comment.
When type is omitted (empty string), Validate currently returns ErrUnknownTestType, which reads like an unsupported value rather than a missing required field. Consider special-casing an empty Type to return ErrMissingTestField with a message like "missing 'type'" to make the config error clearer.
| // Type indicates the test framework to use. | ||
| Type TestType `toml:"type" json:"type" jsonschema:"required,enum=pytest,title=Type,description=Type of test framework (pytest)"` | ||
|
|
||
| // Pytest holds pytest-specific configuration. Required when Type is "pytest". | ||
| Pytest *PytestConfig `toml:"pytest,omitempty" json:"pytest,omitempty" jsonschema:"title=Pytest config,description=Pytest-specific configuration (required when type is pytest)"` | ||
|
|
There was a problem hiding this comment.
This PR introduces new user-facing config fields for test suites (type = "pytest" and the [test-suites.<name>.pytest] subtable). The user guide appears to already have a Test Suites reference page, but it will need to be updated to document the new fields and provide an example for pytest suites.
| matches, err := doublestar.FilepathGlob(absPattern, | ||
| doublestar.WithFilesOnly(), | ||
| doublestar.WithFailOnIOErrors(), | ||
| ) |
There was a problem hiding this comment.
expandGlob uses doublestar.FilepathGlob, which operates on the host filesystem and bypasses the repo’s opctx.FS abstraction. This makes behavior differ under testctx/in-memory FS and is inconsistent with existing usage of fileutils.Glob(fs, pattern, ...). Consider refactoring glob expansion to use fileutils.Glob(env.FS(), ...) (and passing fs/env into BuildNativePytestArgs/expandGlob as needed).
| if !pyprojectExists { | ||
| slog.Warn("No pyproject.toml found; skipping dependency installation", | ||
| slog.String("working-dir", workingDir), | ||
| ) | ||
|
|
||
| return nil | ||
| } |
There was a problem hiding this comment.
installFromPyproject silently skips installation when pyproject.toml is missing, but RunPytestSuite still invokes python -m pytest. In a newly created venv this will reliably fail because pytest won’t be installed. Consider either failing fast with a clear error when pyproject.toml is missing (for install mode 'pyproject') or ensuring pytest (and optionally pip/setuptools/wheel) is installed before running.
Add config-driven pytest test suites with typed TestSuiteConfig. Test suites are defined in [test-suites] with type = "pytest" and a [pytest] subtable specifying working-dir, test-paths (glob-expanded), and extra-args (with {image-path} placeholder substitution).
Rewrite the 'image test' command to resolve test suites from project config, dispatch to the pytest runner, and support --test-suite filtering, --junit-xml output, and automatic image artifact resolution.
Local LISA-specific test runner code is temporarily removed; I have a private branch that will bring it back with the new TOML-driven configuration for local test execution--and plan to follow up with it.