feat(azldev): enable --from-spec to use fmf from local rendered spec (needs skip-file-filter) - #347
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The new --from-spec flow needs follow-up fixes/updates (git prerequisite mismatch, missing unit tests, and CLI/docs regeneration) before it’s reliable and fully integrated.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds a new local-development mode for azldev component test to run TMT plans directly from a component’s locally rendered spec tree (fmf) via a new --from-spec flag, instead of cloning the catalog test metadata source.
Changes:
- Add
--from-specflag and plumb it through test execution settings. - Switch the TMT run directory to either the cloned repo (default) or the rendered spec directory (
--from-spec). - Add validation (
resolveSpecRunDir) to ensure the rendered spec directory contains an fmf root (.fmf/version).
File summaries
| File | Description |
|---|---|
| internal/app/azldev/cmds/component/test.go | Adds --from-spec support, routes TMT execution to rendered spec dirs, and validates presence of fmf metadata. |
Review details
- Files reviewed: 1/1 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.
5671da0 to
a4866aa
Compare
a4866aa to
15c15a9
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The new --from-spec help/comment text hard-codes an incorrect rendered-spec base path (“specs/...”) instead of referencing the configured rendered-specs-dir layout, which is user-facing and misleading.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
internal/app/azldev/cmds/component/test.go:537
- The comment for [resolveSpecRunDir] says it runs from 'specs///', but the rendered spec directory is derived from the project 'rendered-specs-dir' setting (e.g. 'SPECS/c/curl'). Updating this comment avoids baking in an incorrect directory name.
// resolveSpecRunDir validates that the component's rendered spec directory
// exists and carries an fmf root, returning it as the tmt run directory. It
// backs the --from-spec flow, which runs a plan straight from
// specs/<x>/<name>/ instead of cloning the catalog 'source'. tmt writes its run
// artifacts under a separate --workdir-root, so the rendered tree is only read.
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
15c15a9 to
9d33ed3
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The new --from-spec validation and flow can be made more correct and efficient with small fixes (FMF file-type validation and earlier failure before costly venv/pip work).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Lite
9d33ed3 to
12246a1
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The updated help text has an internal inconsistency about git prerequisites when using --from-spec, and should be corrected to avoid misleading users.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
…ec dir Add a --from-spec flag to 'azldev component test' that runs the mapped tmt plan directly from the component's rendered spec directory (specs/<x>/<name>/) instead of cloning the catalog source@ref. Requires render.skip-file-filter = true and a prior 'azldev component render' so the loose fmf tree is preserved. Local inner-loop convenience; cloud/TEE runs still clone.
12246a1 to
0b70a10
Compare
There was a problem hiding this comment.
🟢 Approval recommended
The change is localized, preserves the default behavior, adds explicit validation/error messaging for the new path, and includes targeted unit tests for the new functionality.
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0 new
- Review effort level: Lite
|
Attaching log files for util-linux tmt test runs using spec fmf/plans azldev component test util-linux |
Nan Liu (liunan-ms)
left a comment
Nan Liu (liunan-ms)
left a comment
There was a problem hiding this comment.
LGTM, question: do you consider removing the git clone flow after switching to using rendered files?
There was a problem hiding this comment.
🟡 Changes recommended
The stale-render marker check must be fixed before approval, along with the requested documentation and test updates.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (3)
internal/app/azldev/cmds/component/test.go:559
component renderwritesRENDER_FAILEDwhen the rendered tree is stale or invalid, and other consumers skip directories carrying this marker. This path only checks.fmf/version, so a failed render that leaves old output (for example, an external output rendered without overwrite) can make--from-specexecute stale specs; check the marker before accepting the directory and require a successful re-render.
func resolveSpecRunDir(env *azldev.Env, specDir string) (string, error) {
if specDir == "" {
return "", errors.New(
"'--from-spec' requires a rendered spec directory; ensure 'project.rendered-specs-dir' is set")
}
fmfVersion := filepath.Join(specDir, ".fmf", "version")
internal/app/azldev/cmds/component/test.go:200
- The new execution mode is documented in the generated CLI page, but
docs/user/reference/config/tests.md:53-57still says component TMT execution clones the configured source unconditionally. Please update that user-facing reference to describe--from-specand itsrender.skip-file-filter/prior-render prerequisites so the new behavior is not contradicted.
cmd.Flags().BoolVar(&options.FromSpec, "from-spec", false,
"Run the plan from the component's rendered spec directory (under the configured "+
"'project.rendered-specs-dir', e.g. 'SPECS/c/curl') instead of cloning the catalog "+
"'source'. Requires 'render.skip-file-filter = true' and a prior "+
"'azldev component render'. Local inner-loop convenience; not used by cloud (TEE) runs.")
internal/app/azldev/cmds/component/test.go:200
- This adds a new component-test workflow, but the generated agent guidance still only recommends
azldev adv mock shellin both build-component templates and never mentionsazldev component test/--from-spec. Please update the templates and regenerate the emitted skill output so agents following the repository's documented workflow can discover this feature.
cmd.Flags().BoolVar(&options.FromSpec, "from-spec", false,
"Run the plan from the component's rendered spec directory (under the configured "+
"'project.rendered-specs-dir', e.g. 'SPECS/c/curl') instead of cloning the catalog "+
"'source'. Requires 'render.skip-file-filter = true' and a prior "+
"'azldev component render'. Local inner-loop convenience; not used by cloud (TEE) runs.")
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Lite
| ### Options | ||
|
|
||
| ``` | ||
| --from-spec Run the plan from the component's rendered spec directory (under the configured 'project.rendered-specs-dir', e.g. 'SPECS/c/curl') instead of cloning the catalog 'source'. Requires 'render.skip-file-filter = true' and a prior 'azldev component render'. Local inner-loop convenience; not used by cloud (TEE) runs. |
| require.NoError(t, runOneTMTTest(testEnv.Env, test, settings)) | ||
|
|
||
| ranTMT := false | ||
|
|
||
| for _, args := range testEnv.CommandsExecuted { | ||
| require.NotEmpty(t, args) | ||
| assert.NotEqual(t, "git", filepath.Base(args[0]), | ||
| "--from-spec must not invoke git, but ran: %v", args) | ||
|
|
||
| if filepath.Base(args[0]) == tmtProgram { | ||
| ranTMT = true | ||
| } | ||
| } | ||
|
|
||
| assert.True(t, ranTMT, "expected the tmt run command to be invoked from the spec dir") |
No description provided.