feat(projectconfig): accept [tests] and [test-groups] schema in config

[bhagyapathak]

Adds a parse-only schema for declaring tests and reusable test groups in project TOML, plus matching per-component and per-image test reference lists.

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds support for a “new-shape” test configuration schema by introducing first-class project-level test definitions and groups, and enabling images/components to reference them.

Changes:

• Introduces TestDefinition, TestGroup, TestRef, and ComponentTestsConfig types for the new schema.
• Extends ConfigFile, ImageTestsConfig, and ComponentConfig to carry new test/group references.
• Updates fingerprint decision test to exclude component test-selection metadata from build inputs.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
internal/projectconfig/tests.go	Adds the new schema types for tests, groups, and references.
internal/projectconfig/configfile.go	Adds top-level [tests] and [test-groups] maps to the config shape.
internal/projectconfig/image.go	Allows images to reference tests/groups via ImageTestsConfig.Tests.
internal/projectconfig/component.go	Allows components to reference tests/groups and deep-copies the new field.
internal/projectconfig/fingerprint_test.go	Excludes ComponentConfig.Tests from fingerprint build inputs.

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.

[Copilot]

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 4 comments.

[Nan Liu (liunan-ms)]

The new shape seems a superset of tmt, lisa, and pytest, other fields like required-capabilities, reusable groups, and per-component references. Is this intended as the next-generation replacement for test-suites? If yes, is there a migration path designed?

[reuben olinsky (reubeno)]

Thanks for your patience -- I went through this in detail yesterday; lots of goodness here. I've created a branch based on the work that also adds some additional proposed changes. I'll be sending it out during my day today.

Some of my main comments/questions:

• We need to unify the test-suites and test elements one way or the other.
• There's some additional validation for the test references that I think will be helpful to catch typos/dangling references.
• Can we better constrain the test kinds?
• There are some opportunities for additional unit testing of some of the new code that I think will be worthwhile. Copilot can definitely help with some of it.
• Nested test groups will bring some complexity; how do we imagine they'll be used?

I'll follow up with more details later today.

[Copilot]

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated 7 comments.

[bhagyapathak]

azldev-migration-tests.txt
azldev working on old as well as new test metadata

[Copilot]

Pull request overview

Copilot reviewed 31 out of 32 changed files in this pull request and generated 1 comment.

Suppressed comments (9)

internal/projectconfig/tests.go:356

• [validateLisaSelection] looks for list selector key 'testcaseNames', but configs/docs use 'testcase-names'. As written, 'testcase-names' will be ignored and the test may be rejected as having no selectors.

	if rawNames, ok := lisa["testcaseNames"]; ok {
		hasSelector = true

		if err := validateStringList(rawNames, "lisa.testcaseNames", testName); err != nil {
			return err

internal/projectconfig/tests.go:365

• The error message for missing LISA selectors lists 'testcaseName'/'testcaseNames', but the public TOML keys and other code paths use 'testcase-name'/'testcase-names'. This can confuse users when they follow the docs and still get a validation error.

		return fmt.Errorf(
			"%w: test %#q of type %#q must set at least one LISA selector: criteria, testcaseName, testcaseNames, or name",
			ErrInvalidLisaSelection,
			testName,
			"lisa",

internal/projectconfig/tests.go:430

• [validateSingleLisaCriteria] only allows 'testcaseName'/'testcaseNames' keys, but tests/docs use the hyphenated TOML keys 'testcase-name'/'testcase-names'. This currently makes otherwise-valid configs fail validation (e.g. criteria entries using 'testcase-names').

		"priority":      true,
		"tags":          true,
		"testcaseName":  true,
		"testcaseNames": true,
	}

internal/projectconfig/loader.go:338

• mergeTests reports duplicates using ErrDuplicateTestSuites ("duplicate test suite"). That makes error classification/messages misleading for duplicate [tests] entries and prevents callers/tests from distinguishing duplicate test suites vs duplicate test definitions.

	for testName, testDef := range loadedCfg.Tests {
		if _, ok := resolvedCfg.Tests[testName]; ok {
			return fmt.Errorf("%w: test %#q", ErrDuplicateTestSuites, testName)
		}

internal/projectconfig/loader.go:352

• mergeTestGroups reports duplicates using ErrDuplicateTestSuites ("duplicate test suite"). This makes duplicate [test-groups] failures misleading and prevents errors.Is checks from distinguishing test-suite duplicates from test-group duplicates.

	for groupName, group := range loadedCfg.TestGroups {
		if _, ok := resolvedCfg.TestGroups[groupName]; ok {
			return fmt.Errorf("%w: test group %#q", ErrDuplicateTestSuites, groupName)
		}

internal/projectconfig/tests.go:616

• TestRef.JSONSchemaExtend writes to schema without a nil check. Other JSONSchemaExtend hooks in this file guard against nil schemas; adding the same guard here avoids a potential panic if the schema generator ever calls extensions with a nil schema.

func (TestRef) JSONSchemaExtend(schema *jsonschema.Schema) {
	// Exactly one of name|group: encoded as oneOf with `required` on each and
	// `not` excluding the other, which forbids both `{}` and `{name, group}`.
	schema.OneOf = []*jsonschema.Schema{
		{Required: []string{"name"}, Not: &jsonschema.Schema{Required: []string{"group"}}},

internal/projectconfig/tests.go:449

• validateSingleLisaCriteria's selector switch only treats 'testcaseName' as a valid selector key. Even if the allowed-keys list is updated to accept the TOML-style 'testcase-name', this switch will ignore it and may fail the criteria entry with "must include at least one selector".

		switch key {
		case "name", "area", "category", "testcaseName":
			if !isNonEmptyString(value) {
				return fmt.Errorf(
					"%w: test %#q lisa.criteria[%d].%s must be a non-empty string",

internal/projectconfig/tests.go:467

• validateSingleLisaCriteria's selector switch only treats 'testcaseNames' as the list form. TOML-style 'testcase-names' should also be handled here; otherwise that selector is ignored and may be reported as missing.

			hasSelector = true
		case "tags", "testcaseNames":
			fieldName := "lisa.criteria[" + strconv.Itoa(idx) + "]." + key

			if err := validateStringList(value, fieldName, testName); err != nil {

internal/projectconfig/tests.go:136

• ResolveTestRefs silently ignores invalid TestRef values where neither 'name' nor 'group' is set (or both are set). That can cause missing tests without any error, especially in permissive parsing paths or when configs are constructed programmatically.

	for _, ref := range refs {
		switch {
		case ref.Name != "":

[Copilot]

Pull request overview

Copilot reviewed 31 out of 32 changed files in this pull request and generated no new comments.

Suppressed comments (6)

internal/projectconfig/loader.go:338

• mergeTests reports duplicates using ErrDuplicateTestSuites ("duplicate test suite"), which makes the surfaced error misleading for users and for errors.Is checks (a duplicate [tests] entry looks like a duplicate [test-suites]). Consider introducing a dedicated error for duplicate test definitions (and likewise for duplicate [test-groups]) and using that here.

func mergeTests(resolvedCfg *ProjectConfig, loadedCfg *ConfigFile) error {
	for testName, testDef := range loadedCfg.Tests {
		if _, ok := resolvedCfg.Tests[testName]; ok {
			return fmt.Errorf("%w: test %#q", ErrDuplicateTestSuites, testName)
		}

internal/projectconfig/loader.go:352

• mergeTestGroups reports duplicate group names using ErrDuplicateTestSuites ("duplicate test suite"), which is user-facingly confusing for [test-groups] conflicts and makes errors.Is ambiguous. A dedicated duplicate-test-group error (or a generic duplicate-test-definition error family) would avoid conflating unrelated sections.

func mergeTestGroups(resolvedCfg *ProjectConfig, loadedCfg *ConfigFile) error {
	for groupName, group := range loadedCfg.TestGroups {
		if _, ok := resolvedCfg.TestGroups[groupName]; ok {
			return fmt.Errorf("%w: test group %#q", ErrDuplicateTestSuites, groupName)
		}

internal/projectconfig/tests.go:684

• TestRef.JSONSchemaExtend dereferences schema unconditionally. Other JSONSchemaExtend methods in this file guard against schema == nil, so this one can panic if the schema generator ever invokes it with a nil schema (or if future refactors do).

func (TestRef) JSONSchemaExtend(schema *jsonschema.Schema) {
	// Exactly one of name|group: encoded as oneOf with `required` on each and
	// `not` excluding the other, which forbids both `{}` and `{name, group}`.
	schema.OneOf = []*jsonschema.Schema{
		{Required: []string{"name"}, Not: &jsonschema.Schema{Required: []string{"group"}}},
		{Required: []string{"group"}, Not: &jsonschema.Schema{Required: []string{"name"}}},
	}

internal/app/azldev/cmds/image/test.go:282

• Error wrapping here uses fmt.Errorf("...: %w", err); the repo’s established pattern is "context:\n%w" for readability/consistency when surfacing nested errors.

	resolvedTests, err := cfg.ResolveImageTests(imageConfig)
	if err != nil {
		return nil, nil, fmt.Errorf("resolve image tests: %w", err)
	}

internal/projectconfig/tests.go:612

• validateStringList always wraps failures with ErrInvalidLisaSelection, even when validating non-LISA fields (e.g., pytest.test-paths). This makes errors.Is(err, ErrInvalidLisaSelection) true for pytest config errors, which is misleading and can cause callers/tests to misclassify the failure.

func validateStringList(value any, fieldName string, testName string) error {
	items, ok := value.([]any)
	if !ok || len(items) == 0 {
		return fmt.Errorf(
			"%w: test %#q %s must be a non-empty list of non-empty strings",
			ErrInvalidLisaSelection,
			testName,
			fieldName,
		)
	}

internal/app/azldev/cmds/image/test.go:274

• Error wrapping here uses fmt.Errorf("...: %w", err); elsewhere in this repo the convention is to include a newline before the wrapped error ("context:\n%w") so the wrapped error prints on its own line. This keeps multi-line underlying errors (common with validation errors) readable and consistent.

This issue also appears on line 279 of the same file.

	if len(explicitSelectors) > 0 {
		resolvedTests, err := cfg.ResolveTestSelectors(explicitSelectors)
		if err != nil {
			return nil, nil, fmt.Errorf("resolve test selectors: %w", err)
		}

[Copilot]

Pull request overview

Copilot reviewed 31 out of 32 changed files in this pull request and generated no new comments.

Suppressed comments (8)

internal/projectconfig/loader.go:338

• mergeTests/mergeTestGroups report duplicates using ErrDuplicateTestSuites ("duplicate test suite"), which is misleading for [tests] and [test-groups] collisions and makes it hard to distinguish which section had the duplicate. Consider adding dedicated sentinel errors (e.g., ErrDuplicateTests / ErrDuplicateTestGroups) and using those here.

// mergeTests merges individual test definitions from a loaded config file into the
// resolved config. Duplicate test names are not allowed.
func mergeTests(resolvedCfg *ProjectConfig, loadedCfg *ConfigFile) error {
	for testName, testDef := range loadedCfg.Tests {
		if _, ok := resolvedCfg.Tests[testName]; ok {
			return fmt.Errorf("%w: test %#q", ErrDuplicateTestSuites, testName)
		}

internal/app/azldev/cmds/image/test.go:110

• The '--test-suite' flag help string implies only test/test-group selection, but the implementation uses these values as legacy suite names when the image is configured with legacy 'tests.test-suites'. The flag description should mention this dual meaning.

	cmd.Flags().StringSliceVar(&options.TestSuites, "test-suite", nil,
		"Name of a test or test-group to run (may be repeated; defaults to all tests for the image)")

internal/projectconfig/tests.go:148

• ResolveTestRefs silently ignores invalid TestRef values (neither or both of name/group set) because the switch has no default/error path. This can hide config issues (especially under permissive parsing) and will produce a shorter resolved list without any error.

	for _, ref := range refs {
		switch {
		case ref.Name != "":
			testDef, ok := cfg.Tests[ref.Name]
			if !ok {

internal/projectconfig/tests.go:607

• validateStringList always wraps failures with ErrInvalidLisaSelection, but it is also used for pytest and other validations. This makes the error chain misleading (and can cause callers to match ErrInvalidLisaSelection for non-LISA failures). Consider returning an unwrapped error here and letting the caller wrap with ErrInvalidLisaSelection/ErrInvalidPytestConfig/etc., or pass the intended sentinel error into validateStringList.

func validateStringList(value any, fieldName string, testName string) error {
	items, ok := value.([]any)
	if !ok || len(items) == 0 {
		return fmt.Errorf(
			"%w: test %#q %s must be a non-empty list of non-empty strings",

internal/projectconfig/tests.go:57

• The comment on TestDefinition.Type says the loader accepts unknown values permissively, but TestDefinition.Validate rejects unknown types (ErrUnknownTestType) and ConfigFile.Validate calls Validate during normal loads. This comment should be updated to match the actual validation behavior (or validation should be relaxed if unknown types are intended to be allowed).

	// Type identifies the framework/runner. Required, and constrained to the
	// closed enum in the schema tag at the schema layer. The loader still
	// accepts unknown values permissively; the resolver is the source of truth.

docs/user/reference/config/tests.md:28

• This section says framework-specific subtables are “intentionally not validated”, but the config loader does validate required keys for each framework (e.g., pytest requires working-dir/test-paths; tmt requires plan/source; lisa requires selectors). The docs should reflect the actual minimal validation and the fact that unknown keys are passed through.

Each entry under `[tests.<name>]` describes one configuration of one
runner. Framework-specific options live in a typed subtable
(`pytest`, `lisa`, `tmt`) whose contents are passed through
to the runner; their internal schemas are intentionally not validated
by azldev so frameworks can evolve independently.

internal/app/azldev/cmds/image/test.go:65

• The help text says '--test-suite' selects test names or test-group names, but resolveImageTestsToRun treats these as legacy test suite names when the image has no new-style 'tests.tests' entries. This is confusing for users running legacy-configured images.

This issue also appears on line 108 of the same file.

By default, all tests associated with the named image are run. Use
--test-suite to select specific test names or test-group names (may be repeated).

internal/app/azldev/cmds/component/query.go:97

• Error messages in this codebase consistently use %#q when interpolating strings so the value is clearly quoted/escaped. This new error uses %q for the component name; switch to %#q for consistency with the repo’s error-message formatting guideline.

			resolvedTests, err := cfg.ResolveComponentTests(comp.GetConfig())
			if err != nil {
				return nil, fmt.Errorf("failed to resolve tests for component %q:\n%w", comp.GetName(), err)
			}

[Copilot]

Pull request overview

Copilot reviewed 31 out of 31 changed files in this pull request and generated no new comments.

Suppressed comments (1)

docs/user/reference/config/images.md:39

• Project config validation now rejects images that set more than one “delivery kind” capability (machine-bootable, container, wsl, installer-media) to true (see validateImageCapabilities). This section documents the fields but doesn’t mention the mutual-exclusion rule, so users can hit validation errors without guidance.

| WSL | `wsl` | bool | unset | Whether the image runs under the Windows Subsystem for Linux runtime |
| Installer Media | `installer-media` | bool | unset | Whether the image is installer media (e.g. an ISO) that installs another OS, rather than a directly runnable end-state image |
| FIPS Enabled | `fips-enabled` | bool | unset | Whether the image is built or configured to run in FIPS mode |
| CVM | `cvm` | bool | unset | Whether the image supports running as a Confidential VM (CVM) |

[Copilot]

Pull request overview

Copilot reviewed 33 out of 33 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 33 out of 33 changed files in this pull request and generated no new comments.

Suppressed comments (1)

internal/projectconfig/tests.go:701

• [TestRef.JSONSchemaExtend] dereferences schema unconditionally (schema.OneOf = …). Other JSONSchemaExtend methods in this file guard against nil schemas; adding the same guard here avoids a potential panic if a future caller (or a jsonschema edge case) ever invokes the hook with a nil schema.

func (TestRef) JSONSchemaExtend(schema *jsonschema.Schema) {
	// Exactly one of name|group: encoded as oneOf with `required` on each and
	// `not` excluding the other, which forbids both `{}` and `{name, group}`.
	schema.OneOf = []*jsonschema.Schema{
		{Required: []string{"name"}, Not: &jsonschema.Schema{Required: []string{"group"}}},
		{Required: []string{"group"}, Not: &jsonschema.Schema{Required: []string{"name"}}},
	}