Skip to content

Commit 106ece2

Browse files
tobiasb-msCopilotCopilottobiasb_microsoft
authored
feat: add ability to skip check section for a specific component (#437)
* feat: add ability to skip check section for a specific component * Update docs/reference/overlays.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix: remove commas from jsonschema descriptions and use single % for %check (#438) * Initial plan * fix: remove commas from jsonschema descriptions and use single % for %check Co-authored-by: tobiasb_microsoft <115835401+tobiasb_microsoft@users.noreply.github.com> * test: update snapshots for schema changes Co-authored-by: tobiasb_microsoft <115835401+tobiasb_microsoft@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: tobiasb_microsoft <115835401+tobiasb_microsoft@users.noreply.github.com> * chore: rename reason to skip_reason for clarity and future-proofing * fix: change wording and use skip_reason in overlays.md * chore: move check config documentation into a new file because it is not an overlay --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: tobiasb_microsoft <115835401+tobiasb_microsoft@users.noreply.github.com>
1 parent 533474c commit 106ece2

10 files changed

Lines changed: 428 additions & 0 deletions

File tree

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
# Customized Build Options
2+
There are options to customize an overall component build.
3+
4+
## Customized Build Types
5+
6+
### Check Section
7+
To disable the `%check` section of a spec when there are known test issues, use the `check` build configuration option:
8+
9+
```toml
10+
[components.mypackage.build]
11+
check = {skip = true, skip_reason = "Tests require network access unavailable in build environment"}
12+
```
13+
14+
| Field | Type | Required | Description |
15+
|-------|------|----------|-------------|
16+
| `skip` | boolean | No | When `true`, disables the `%check` section (defaults to `false` when omitted) |
17+
| `skip_reason` | string | Conditional (when skip=true) | Justification for why the check section is being skipped |
18+
19+
When `skip` is `true`, azldev prevents the code in the `%check` section from running.
20+
21+
> **Note:** The `skip_reason` field is required when `skip` is `true`. This ensures that disabling tests is always documented and traceable.

internal/app/azldev/core/sources/sourceprep.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,15 @@ func (p *sourcePreparerImpl) postProcessSources(
224224
}
225225
}
226226

227+
// Apply check skip overlay if configured.
228+
checkSkipOverlays := synthesizeCheckSkipOverlays(component.GetConfig().Build.Check)
229+
for _, overlay := range checkSkipOverlays {
230+
err = ApplyOverlayToSources(p.dryRunnable, p.fs, overlay, sourcesDirPath, absSpecPath)
231+
if err != nil {
232+
return fmt.Errorf("failed to apply check skip overlay to sources for component %#q:\n%w", component.GetName(), err)
233+
}
234+
}
235+
227236
// Finally, apply the file header overlay.
228237
for _, overlay := range headerOverlay {
229238
err = ApplyOverlayToSources(p.dryRunnable, p.fs, overlay, sourcesDirPath, absSpecPath)
@@ -288,6 +297,27 @@ func generateFileHeaderOverlay() []projectconfig.ComponentOverlay {
288297
}
289298
}
290299

300+
// synthesizeCheckSkipOverlays generates overlays to disable the %check section if configured.
301+
// When check.skip is true, it prepends an 'exit 0' to the %check section with a comment
302+
// explaining why the section was disabled.
303+
func synthesizeCheckSkipOverlays(checkConfig projectconfig.CheckConfig) []projectconfig.ComponentOverlay {
304+
if !checkConfig.Skip {
305+
return nil
306+
}
307+
308+
return []projectconfig.ComponentOverlay{
309+
{
310+
Type: projectconfig.ComponentOverlayPrependSpecLines,
311+
SectionName: "%check",
312+
Lines: []string{
313+
"# Check section disabled: " + checkConfig.SkipReason,
314+
"exit 0",
315+
"",
316+
},
317+
},
318+
}
319+
}
320+
291321
func findSpecInDir(
292322
fs opctx.FS, component components.Component, dirPath string,
293323
) (string, error) {

internal/app/azldev/core/sources/sourceprep_test.go

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,3 +275,107 @@ func TestGenerateMacrosFileContents_FullConfig(t *testing.T) {
275275
// Verify file ends with newline.
276276
assert.Equal(t, '\n', rune(contents[len(contents)-1]))
277277
}
278+
279+
func TestPrepareSources_CheckSkip(t *testing.T) {
280+
const outputSpecPath = testOutputDir + "/test-component.spec"
281+
282+
ctrl := gomock.NewController(t)
283+
component := components_testutils.NewMockComponent(ctrl)
284+
sourceManager := sourceproviders_test.NewMockSourceManager(ctrl)
285+
ctx := testctx.NewCtx()
286+
287+
component.EXPECT().GetName().AnyTimes().Return("test-component")
288+
component.EXPECT().GetConfig().AnyTimes().Return(&projectconfig.ComponentConfig{
289+
Build: projectconfig.ComponentBuildConfig{
290+
Check: projectconfig.CheckConfig{
291+
Skip: true,
292+
SkipReason: "Tests require network access",
293+
},
294+
},
295+
})
296+
sourceManager.EXPECT().FetchComponent(gomock.Any(), component, testOutputDir).DoAndReturn(
297+
func(_ interface{}, _ interface{}, outputDir string) error {
298+
// Create the expected spec file with a %check section.
299+
specContent := `Name: test-component
300+
Version: 1.0
301+
Release: 1
302+
Summary: Test component
303+
304+
%check
305+
make test
306+
`
307+
308+
return fileutils.WriteFile(ctx.FS(), outputSpecPath, []byte(specContent), 0o644)
309+
},
310+
)
311+
312+
preparer, err := sources.NewPreparer(sourceManager, ctx.FS(), ctx, ctx)
313+
require.NoError(t, err)
314+
err = preparer.PrepareSources(ctx, component, testOutputDir, true /*applyOverlays?*/)
315+
require.NoError(t, err)
316+
317+
// Verify spec has check skip prepended.
318+
specContents, err := fileutils.ReadFile(ctx.FS(), outputSpecPath)
319+
require.NoError(t, err)
320+
321+
specStr := string(specContents)
322+
323+
assert.Contains(t, specStr, "# Check section disabled: Tests require network access")
324+
assert.Contains(t, specStr, "exit 0")
325+
326+
// Verify exit 0 appears after %check and before original content.
327+
checkIdx := strings.Index(specStr, "%check")
328+
exitIdx := strings.Index(specStr, "exit 0")
329+
makeTestIdx := strings.Index(specStr, "make test")
330+
331+
assert.Greater(t, checkIdx, -1, "%check should be present")
332+
assert.Greater(t, exitIdx, checkIdx, "exit 0 should come after %check")
333+
assert.Greater(t, makeTestIdx, exitIdx, "make test should come after exit 0")
334+
}
335+
336+
func TestPrepareSources_CheckSkipDisabled(t *testing.T) {
337+
const outputSpecPath = testOutputDir + "/test-component.spec"
338+
339+
ctrl := gomock.NewController(t)
340+
component := components_testutils.NewMockComponent(ctrl)
341+
sourceManager := sourceproviders_test.NewMockSourceManager(ctrl)
342+
ctx := testctx.NewCtx()
343+
344+
component.EXPECT().GetName().AnyTimes().Return("test-component")
345+
component.EXPECT().GetConfig().AnyTimes().Return(&projectconfig.ComponentConfig{
346+
Build: projectconfig.ComponentBuildConfig{
347+
Check: projectconfig.CheckConfig{
348+
Skip: false,
349+
},
350+
},
351+
})
352+
sourceManager.EXPECT().FetchComponent(gomock.Any(), component, testOutputDir).DoAndReturn(
353+
func(_ interface{}, _ interface{}, outputDir string) error {
354+
// Create the expected spec file with a %check section.
355+
specContent := `Name: test-component
356+
Version: 1.0
357+
Release: 1
358+
Summary: Test component
359+
360+
%check
361+
make test
362+
`
363+
364+
return fileutils.WriteFile(ctx.FS(), outputSpecPath, []byte(specContent), 0o644)
365+
},
366+
)
367+
368+
preparer, err := sources.NewPreparer(sourceManager, ctx.FS(), ctx, ctx)
369+
require.NoError(t, err)
370+
err = preparer.PrepareSources(ctx, component, testOutputDir, true /*applyOverlays?*/)
371+
require.NoError(t, err)
372+
373+
// Verify spec does NOT have check skip prepended.
374+
specContents, err := fileutils.ReadFile(ctx.FS(), outputSpecPath)
375+
require.NoError(t, err)
376+
377+
specStr := string(specContents)
378+
379+
assert.NotContains(t, specStr, "# Check section disabled")
380+
assert.NotContains(t, specStr, "exit 0")
381+
}

internal/projectconfig/build.go

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,32 @@
33

44
package projectconfig
55

6+
import (
7+
"errors"
8+
"fmt"
9+
)
10+
11+
// CheckConfig encapsulates configuration for the %check section of a spec file.
12+
type CheckConfig struct {
13+
// Skip indicates whether the %check section should be disabled for this component.
14+
Skip bool `toml:"skip,omitempty" json:"skip,omitempty" jsonschema:"title=Skip check,description=Disables the %check section by prepending 'exit 0' when set to true"`
15+
// SkipReason provides a required justification when Skip is true.
16+
SkipReason string `toml:"skip_reason,omitempty" json:"skipReason,omitempty" jsonschema:"title=Skip reason,description=Required justification for skipping the %check section"`
17+
}
18+
19+
// Validate checks that required fields are set when Skip is true.
20+
func (c *CheckConfig) Validate() error {
21+
if !c.Skip {
22+
return nil
23+
}
24+
25+
if c.SkipReason == "" {
26+
return errors.New("check.skip_reason is required when check.skip is true")
27+
}
28+
29+
return nil
30+
}
31+
632
// Encapsulates configuration for building a component. Configuration for how to acquire
733
// or prepare the sources for a component are out of scope.
834
type ComponentBuildConfig struct {
@@ -12,4 +38,15 @@ type ComponentBuildConfig struct {
1238
Without []string `toml:"without,omitempty" json:"without,omitempty" jsonschema:"title=Without options,description='without' options to pass to the builder."`
1339
// Macro definitions.
1440
Defines map[string]string `toml:"defines,omitempty" json:"defines,omitempty" jsonschema:"title=Macro definitions,description=Macro definitions to pass to the builder."`
41+
// Check section configuration.
42+
Check CheckConfig `toml:"check,omitempty" json:"check,omitempty" jsonschema:"title=Check configuration,description=Configuration for the %check section"`
43+
}
44+
45+
// Validate checks that the build configuration is valid.
46+
func (c *ComponentBuildConfig) Validate() error {
47+
if err := c.Check.Validate(); err != nil {
48+
return fmt.Errorf("invalid build configuration:\n%w", err)
49+
}
50+
51+
return nil
1552
}
Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT License.
3+
4+
package projectconfig_test
5+
6+
import (
7+
"testing"
8+
9+
"github.com/gim-home/azldev-preview/internal/projectconfig"
10+
"github.com/stretchr/testify/assert"
11+
"github.com/stretchr/testify/require"
12+
)
13+
14+
func TestCheckConfig_Validate(t *testing.T) {
15+
testCases := []struct {
16+
name string
17+
config projectconfig.CheckConfig
18+
errorExpected bool
19+
errorContains string
20+
}{
21+
{
22+
name: "empty config is valid",
23+
config: projectconfig.CheckConfig{},
24+
errorExpected: false,
25+
},
26+
{
27+
name: "skip false without reason is valid",
28+
config: projectconfig.CheckConfig{
29+
Skip: false,
30+
},
31+
errorExpected: false,
32+
},
33+
{
34+
name: "skip true with reason is valid",
35+
config: projectconfig.CheckConfig{
36+
Skip: true,
37+
SkipReason: "Tests require network access",
38+
},
39+
errorExpected: false,
40+
},
41+
{
42+
name: "skip true without reason is invalid",
43+
config: projectconfig.CheckConfig{
44+
Skip: true,
45+
},
46+
errorExpected: true,
47+
errorContains: "skip_reason",
48+
},
49+
}
50+
51+
for _, testCase := range testCases {
52+
t.Run(testCase.name, func(t *testing.T) {
53+
err := testCase.config.Validate()
54+
55+
if testCase.errorExpected {
56+
require.Error(t, err)
57+
58+
if testCase.errorContains != "" {
59+
assert.Contains(t, err.Error(), testCase.errorContains)
60+
}
61+
62+
return
63+
}
64+
65+
require.NoError(t, err)
66+
})
67+
}
68+
}
69+
70+
func TestComponentBuildConfig_Validate(t *testing.T) {
71+
testCases := []struct {
72+
name string
73+
config projectconfig.ComponentBuildConfig
74+
errorExpected bool
75+
errorContains string
76+
}{
77+
{
78+
name: "empty config is valid",
79+
config: projectconfig.ComponentBuildConfig{},
80+
errorExpected: false,
81+
},
82+
{
83+
name: "config with with/without/defines is valid",
84+
config: projectconfig.ComponentBuildConfig{
85+
With: []string{"feature1"},
86+
Without: []string{"feature2"},
87+
Defines: map[string]string{"key": "value"},
88+
},
89+
errorExpected: false,
90+
},
91+
{
92+
name: "config with valid check skip is valid",
93+
config: projectconfig.ComponentBuildConfig{
94+
Check: projectconfig.CheckConfig{
95+
Skip: true,
96+
SkipReason: "Tests require network access",
97+
},
98+
},
99+
errorExpected: false,
100+
},
101+
{
102+
name: "config with invalid check skip propagates error",
103+
config: projectconfig.ComponentBuildConfig{
104+
Check: projectconfig.CheckConfig{
105+
Skip: true,
106+
// Missing SkipReason
107+
},
108+
},
109+
errorExpected: true,
110+
errorContains: "skip_reason",
111+
},
112+
}
113+
114+
for _, testCase := range testCases {
115+
t.Run(testCase.name, func(t *testing.T) {
116+
err := testCase.config.Validate()
117+
118+
if testCase.errorExpected {
119+
require.Error(t, err)
120+
121+
if testCase.errorContains != "" {
122+
assert.Contains(t, err.Error(), testCase.errorContains)
123+
}
124+
125+
return
126+
}
127+
128+
require.NoError(t, err)
129+
})
130+
}
131+
}

internal/projectconfig/configfile.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,12 @@ func (f ConfigFile) Validate() error {
6262
return fmt.Errorf("invalid overlay %d for component %#q:\n%w", i+1, componentName, err)
6363
}
6464
}
65+
66+
// Validate build configuration.
67+
err := component.Build.Validate()
68+
if err != nil {
69+
return fmt.Errorf("invalid build config for component %#q:\n%w", componentName, err)
70+
}
6571
}
6672

6773
return nil

0 commit comments

Comments
 (0)