Tests for validating recipes

[canercidam]

No description provided.

[claude]

Claude finished @canercidam's task in 2m 12s —— View job

---

PR #373 Review: Tests for validating recipes

• Gather context and read changed files
• Analyze code quality and patterns
• Post review feedback

---

Summary

This PR makes four changes:

1. Extracts GetBaseRecipes() from main.go into playground/manifest.go to avoid duplicating the recipe list between main.go and test code.
2. Fixes ValidateRecipe to create a temp output directory before calling recipe.Apply(), which requires ExContext.Output to be set.
3. Adds TestValidateBaseRecipes to validate all built-in recipes pass validation.
4. Adds TestValidateShippedCustomRecipes to validate all shipped custom recipes pass validation (filtering environment-specific host_path errors).

The changes are well-structured. A few observations below.

Issues

1. TestValidateShippedCustomRecipes uses runtime.Caller for repo root discovery

playground/custom_recipes_test.go:17-24 — Using runtime.Caller(0) to find the repo root is fragile if the test binary is run from a different location or the file is moved. A more robust approach is os.Getwd() combined with walking up to find go.mod, or using go list -m -json to find the module root. This works today but is worth noting.

2. TestValidateShippedCustomRecipes silently passes when host_path errors are the only errors

playground/custom_recipes_test.go:488-494 — The filter for host_path does not exist errors is reasonable for CI, but it means a recipe with only host_path errors will appear to pass validation fully. Consider logging filtered errors as t.Log so they're visible in verbose test output:

for _, e := range result.Errors {
    if strings.Contains(e, "host_path does not exist") {
        t.Logf("filtered environment-specific error: %s", e)
    } else {
        errs = append(errs, e)
    }
}

3. Pre-existing: Duplicate mount validation in Manifest.Validate

playground/manifest.go:332-358 — Not introduced by this PR, but the "validate that the mounts are correct" loop is duplicated (lines 332-344 and 346-358 are identical). Worth cleaning up in a follow-up.

Minor notes

• The recipe_yaml_test.go changes are mechanical []Recipe{&L1Recipe{}} → GetBaseRecipes() substitutions. These are correct and make tests more comprehensive by including all base recipes in the parse validation.
• The validate CLI command now correctly reports "recipe file '%s' not found" instead of searching base recipes by name (which was inconsistent since validate only accepts YAML file paths).

[claude]

The host_path error filter silently hides these errors. Consider adding t.Logf for filtered errors so they're visible in verbose test output (go test -v), making it easier to notice when a recipe has environment-specific issues:

Suggested change

	// Filter host_path errors (environment-specific)
	var errs []string
	for _, e := range result.Errors {
	if !strings.Contains(e, "host_path does not exist") {
	errs = append(errs, e)
	}
	}
	var errs []string
	for _, e := range result.Errors {
	if strings.Contains(e, "host_path does not exist") {
	t.Logf("filtered environment-specific error: %s", e)
	} else {
	errs = append(errs, e)
	}
	}