feat(config)!: remove legacy test-suites; fix pytest working-dir resolution - #350
feat(config)!: remove legacy test-suites; fix pytest working-dir resolution#350bhagyapathak wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Schema snapshots and relative pytest path handling remain unresolved, with stale configuration documentation also requiring updates.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Removes legacy [test-suites] support in favor of project-level [tests] and [test-groups] configuration.
Changes:
- Removes legacy suite schema, loading, validation, and execution.
- Updates image test selection, runners, and related tests.
- Updates generated schema and CLI documentation.
File summaries
| File | Reviewed changes |
|---|---|
schemas/azldev.schema.json |
Removes legacy schema definitions. Critical (3 votes): regenerate scenario snapshots. |
internal/projectconfig/testsuite.go |
Removes legacy suite metadata while retaining shared runner configuration. |
internal/projectconfig/testsuite_test.go |
Updates test references and removes legacy suite coverage. |
internal/projectconfig/tests.go |
Updates test path handling. Moderate (3 votes): preserve source-directory provenance for relative pytest paths. |
internal/projectconfig/project.go |
Removes top-level test-suite configuration and validation. |
internal/projectconfig/loader.go |
Removes legacy suite loading and merging. |
internal/projectconfig/loader_test.go |
Updates configuration loading tests. |
internal/projectconfig/image.go |
Removes legacy image test references. |
internal/projectconfig/configfile.go |
Removes legacy configuration validation. |
internal/projectconfig/configfile_test.go |
Removes legacy validation and deprecation-warning coverage. |
internal/app/azldev/cmds/image/test.go |
Uses new test selectors and runners. Nit (3 votes): update stale configuration documentation and agent skill references. |
internal/app/azldev/cmds/image/test_internal_test.go |
Updates selector and resolver tests. |
internal/app/azldev/cmds/image/list_test.go |
Updates image test fixtures. |
internal/app/azldev/cmds/image/lisarunner.go |
Removes the legacy LISA suite runner. |
docs/user/reference/cli/azldev_image_test.md |
Updates image-test CLI documentation. |
Review details
- Files reviewed: 15/15 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
e678751 to
ae42d08
Compare
ae42d08 to
1deeda3
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Fix the schema freshness issue and add regression coverage for included-config pytest paths.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
internal/projectconfig/tests.go:99
- This provenance assignment is the behavior that makes relative
working-dirvalues from included config files resolve against the file that defines them, but the added regression test only loads the root config file. Please add a test with a root file including a pytest definition from a different directory; otherwise a wrongloadedCfg.dirpropagation could pass the current tests.
result.dir = referenceDir
- Files reviewed: 22/22 changed files
- Comments generated: 1
- Review effort level: Lite
bhagyapathak
changed the title
| resolvedTests, err := resolveImageTestsToRun(cfg, imageConfig, options.TestSelectors) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| if len(resolvedTests) == 0 && len(legacySuiteNames) == 0 { | ||
| if len(resolvedTests) == 0 { | ||
| slog.Warn("No tests to run for image", slog.String("image", options.ImageName)) | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| return runImageTests(env, cfg, imageConfig, options, resolvedTests, legacySuiteNames) | ||
| return runImageTests(env, imageConfig, options, resolvedTests) | ||
| } |
There was a problem hiding this comment.
With --permissive-config, removing the test-suites fields causes both top-level [test-suites] definitions and images.<name>.tests.test-suites references to be ignored as unknown fields. This leaves resolvedTests empty, and returns success without executing anything. Would this be an expected behavior?
| | Working directory | `working-dir` | string | No | Directory used as pytest's CWD. Relative paths are resolved against the config file's directory. Required when `install` is `pyproject` or `requirements`. | | ||
| | Test paths | `test-paths` | array of strings | No | Test file paths or directories passed to pytest as positional arguments. Each entry is glob-expanded (including recursive `**`) relative to `working-dir`. Patterns that match nothing are passed through unchanged so pytest reports the failure. | | ||
| | Extra args | `extra-args` | array of strings | No | Additional arguments passed to pytest verbatim, after placeholder substitution. See [Placeholders](#placeholders). | | ||
| | Install mode | `install` | string | No | How dependencies are installed into the venv. One of `pyproject`, `requirements`, or `none` (default). | |
There was a problem hiding this comment.
Deleting this page also removes the only complete doc for the pytest fields that azldev still validates and consumes.
It'd be better to move some of this doc into tests.md and add a concise legacy migration table, including [test-suites.NAME] → [tests.NAME], tests.test-suites → tests.tests, and the LISA framework/test-cases → source/selector changes. Otherwise users affected by this breaking removal will not have enough information to migrate safely.
No description provided.