Skip to content

Commit 5a47eb3

Browse files
lpcoxCopilot
andcommitted
fix: update tests for copilot pre-flight diagnostic step
Update all Copilot engine tests to account for the new pre-flight diagnostic step (2 steps instead of 1). Extract a shared helper for finding the Copilot execution step, and regenerate WASM golden files to include the new step. Fixes: - TestFirewallArgsInCopilotEngine - TestFirewallBlockedDomainsInCopilotEngine - TestFirewallLogLevelInCopilotEngine - TestChrootModeInAWFContainer - TestChrootModeEnvFlags - TestMCPScriptsWithFirewallIncludesHostDockerInternal - TestEngineAWFEnableApiProxy - TestWasmGolden_CompileFixtures (golden files) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent de9842f commit 5a47eb3

9 files changed

Lines changed: 76 additions & 109 deletions

File tree

pkg/workflow/enable_api_proxy_test.go

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,35 @@ import (
55
"testing"
66
)
77

8+
func requireCopilotPreflightAndExecutionSteps(t *testing.T, steps []GitHubActionStep) (string, string) {
9+
t.Helper()
10+
11+
if len(steps) != 2 {
12+
t.Fatalf("Expected 2 execution steps (preflight + execution), got %d", len(steps))
13+
}
14+
15+
preflightContent := strings.Join(steps[0], "\n")
16+
if !strings.Contains(preflightContent, "Copilot pre-flight diagnostic") {
17+
t.Fatalf("Expected first Copilot step to be the pre-flight diagnostic, got:\n%s", preflightContent)
18+
}
19+
if !strings.Contains(preflightContent, "id: copilot-preflight") {
20+
t.Fatalf("Expected pre-flight step to have id 'copilot-preflight', got:\n%s", preflightContent)
21+
}
22+
if !strings.Contains(preflightContent, "copilot_preflight_diagnostic.sh") {
23+
t.Fatalf("Expected pre-flight step to run the diagnostic script, got:\n%s", preflightContent)
24+
}
25+
26+
executionContent := strings.Join(steps[1], "\n")
27+
if !strings.Contains(executionContent, "Execute GitHub Copilot CLI") {
28+
t.Fatalf("Expected second Copilot step to execute the CLI, got:\n%s", executionContent)
29+
}
30+
if !strings.Contains(executionContent, "id: agentic_execution") {
31+
t.Fatalf("Expected execution step to have id 'agentic_execution', got:\n%s", executionContent)
32+
}
33+
34+
return preflightContent, executionContent
35+
}
36+
837
// TestEngineAWFEnableApiProxy tests that engines with LLM gateway support
938
// include --enable-api-proxy flag in AWF commands.
1039
func TestEngineAWFEnableApiProxy(t *testing.T) {
@@ -51,11 +80,7 @@ func TestEngineAWFEnableApiProxy(t *testing.T) {
5180
engine := NewCopilotEngine()
5281
steps := engine.GetExecutionSteps(workflowData, "test.log")
5382

54-
if len(steps) == 0 {
55-
t.Fatal("Expected at least one execution step")
56-
}
57-
58-
stepContent := strings.Join(steps[0], "\n")
83+
_, stepContent := requireCopilotPreflightAndExecutionSteps(t, steps)
5984

6085
if !strings.Contains(stepContent, "--enable-api-proxy") {
6186
t.Error("Expected Copilot AWF command to contain '--enable-api-proxy' flag")

pkg/workflow/firewall_args_test.go

Lines changed: 9 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,7 @@ func TestFirewallArgsInCopilotEngine(t *testing.T) {
2727
engine := NewCopilotEngine()
2828
steps := engine.GetExecutionSteps(workflowData, "test.log")
2929

30-
if len(steps) == 0 {
31-
t.Fatal("Expected at least one execution step")
32-
}
33-
34-
stepContent := strings.Join(steps[0], "\n")
30+
_, stepContent := requireCopilotPreflightAndExecutionSteps(t, steps)
3531

3632
// Check that the command contains awf (AWF v0.15.0+ uses chroot mode by default)
3733
if !strings.Contains(stepContent, "sudo -E awf") {
@@ -69,11 +65,7 @@ func TestFirewallArgsInCopilotEngine(t *testing.T) {
6965
engine := NewCopilotEngine()
7066
steps := engine.GetExecutionSteps(workflowData, "test.log")
7167

72-
if len(steps) == 0 {
73-
t.Fatal("Expected at least one execution step")
74-
}
75-
76-
stepContent := strings.Join(steps[0], "\n")
68+
_, stepContent := requireCopilotPreflightAndExecutionSteps(t, steps)
7769

7870
// Check that custom args are included
7971
if !strings.Contains(stepContent, "--custom-arg") {
@@ -111,11 +103,7 @@ func TestFirewallArgsInCopilotEngine(t *testing.T) {
111103
engine := NewCopilotEngine()
112104
steps := engine.GetExecutionSteps(workflowData, "test.log")
113105

114-
if len(steps) == 0 {
115-
t.Fatal("Expected at least one execution step")
116-
}
117-
118-
stepContent := strings.Join(steps[0], "\n")
106+
_, stepContent := requireCopilotPreflightAndExecutionSteps(t, steps)
119107

120108
// Check that args with spaces are present (they should be escaped)
121109
if !strings.Contains(stepContent, "--message") {
@@ -144,11 +132,7 @@ func TestFirewallArgsInCopilotEngine(t *testing.T) {
144132
engine := NewCopilotEngine()
145133
steps := engine.GetExecutionSteps(workflowData, "test.log")
146134

147-
if len(steps) == 0 {
148-
t.Fatal("Expected at least one execution step")
149-
}
150-
151-
stepContent := strings.Join(steps[0], "\n")
135+
_, stepContent := requireCopilotPreflightAndExecutionSteps(t, steps)
152136

153137
// Check that AWF is used for transparent host access (AWF v0.15.0+)
154138
// Chroot mode is now the default, so no --enable-chroot flag is needed
@@ -178,11 +162,7 @@ func TestFirewallArgsInCopilotEngine(t *testing.T) {
178162
engine := NewCopilotEngine()
179163
steps := engine.GetExecutionSteps(workflowData, "test.log")
180164

181-
if len(steps) == 0 {
182-
t.Fatal("Expected at least one execution step")
183-
}
184-
185-
stepContent := strings.Join(steps[0], "\n")
165+
_, stepContent := requireCopilotPreflightAndExecutionSteps(t, steps)
186166

187167
// Check that --image-tag is included with default version (without v prefix)
188168
expectedImageTag := "--image-tag " + strings.TrimPrefix(string(constants.DefaultFirewallVersion), "v")
@@ -209,11 +189,7 @@ func TestFirewallArgsInCopilotEngine(t *testing.T) {
209189
engine := NewCopilotEngine()
210190
steps := engine.GetExecutionSteps(workflowData, "test.log")
211191

212-
if len(steps) == 0 {
213-
t.Fatal("Expected at least one execution step")
214-
}
215-
216-
stepContent := strings.Join(steps[0], "\n")
192+
_, stepContent := requireCopilotPreflightAndExecutionSteps(t, steps)
217193

218194
// Check that --image-tag is included with custom version (without v prefix)
219195
expectedImageTag := "--image-tag " + strings.TrimPrefix(customVersion, "v")
@@ -245,11 +221,7 @@ func TestFirewallArgsInCopilotEngine(t *testing.T) {
245221
engine := NewCopilotEngine()
246222
steps := engine.GetExecutionSteps(workflowData, "test.log")
247223

248-
if len(steps) == 0 {
249-
t.Fatal("Expected at least one execution step")
250-
}
251-
252-
stepContent := strings.Join(steps[0], "\n")
224+
_, stepContent := requireCopilotPreflightAndExecutionSteps(t, steps)
253225

254226
// Check that --ssl-bump flag is included
255227
if !strings.Contains(stepContent, "--ssl-bump") {
@@ -275,11 +247,7 @@ func TestFirewallArgsInCopilotEngine(t *testing.T) {
275247
engine := NewCopilotEngine()
276248
steps := engine.GetExecutionSteps(workflowData, "test.log")
277249

278-
if len(steps) == 0 {
279-
t.Fatal("Expected at least one execution step")
280-
}
281-
282-
stepContent := strings.Join(steps[0], "\n")
250+
_, stepContent := requireCopilotPreflightAndExecutionSteps(t, steps)
283251

284252
// Check that --ssl-bump flag is included
285253
if !strings.Contains(stepContent, "--ssl-bump") {
@@ -314,11 +282,7 @@ func TestFirewallArgsInCopilotEngine(t *testing.T) {
314282
engine := NewCopilotEngine()
315283
steps := engine.GetExecutionSteps(workflowData, "test.log")
316284

317-
if len(steps) == 0 {
318-
t.Fatal("Expected at least one execution step")
319-
}
320-
321-
stepContent := strings.Join(steps[0], "\n")
285+
_, stepContent := requireCopilotPreflightAndExecutionSteps(t, steps)
322286

323287
// Check that --ssl-bump flag is NOT included
324288
if strings.Contains(stepContent, "--ssl-bump") {

pkg/workflow/firewall_blocked_domains_test.go

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,7 @@ func TestFirewallBlockedDomainsInCopilotEngine(t *testing.T) {
2929
engine := NewCopilotEngine()
3030
steps := engine.GetExecutionSteps(workflowData, "test.log")
3131

32-
assert.NotEmpty(t, steps, "Expected at least one execution step")
33-
34-
stepContent := strings.Join(steps[0], "\n")
32+
_, stepContent := requireCopilotPreflightAndExecutionSteps(t, steps)
3533

3634
// Verify --allow-domains is present
3735
assert.Contains(t, stepContent, "--allow-domains", "Expected command to contain '--allow-domains'")
@@ -61,9 +59,7 @@ func TestFirewallBlockedDomainsInCopilotEngine(t *testing.T) {
6159
engine := NewCopilotEngine()
6260
steps := engine.GetExecutionSteps(workflowData, "test.log")
6361

64-
assert.NotEmpty(t, steps, "Expected at least one execution step")
65-
66-
stepContent := strings.Join(steps[0], "\n")
62+
_, stepContent := requireCopilotPreflightAndExecutionSteps(t, steps)
6763

6864
// Verify --allow-domains is present
6965
assert.Contains(t, stepContent, "--allow-domains", "Expected command to contain '--allow-domains'")
@@ -90,9 +86,7 @@ func TestFirewallBlockedDomainsInCopilotEngine(t *testing.T) {
9086
engine := NewCopilotEngine()
9187
steps := engine.GetExecutionSteps(workflowData, "test.log")
9288

93-
assert.NotEmpty(t, steps, "Expected at least one execution step")
94-
95-
stepContent := strings.Join(steps[0], "\n")
89+
_, stepContent := requireCopilotPreflightAndExecutionSteps(t, steps)
9690

9791
// Verify --block-domains is present
9892
assert.Contains(t, stepContent, "--block-domains", "Expected command to contain '--block-domains'")

pkg/workflow/firewall_log_level_test.go

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -137,11 +137,7 @@ func TestFirewallLogLevelInCopilotEngine(t *testing.T) {
137137
engine := NewCopilotEngine()
138138
steps := engine.GetExecutionSteps(workflowData, "test.log")
139139

140-
if len(steps) == 0 {
141-
t.Fatal("Expected at least one execution step")
142-
}
143-
144-
stepContent := strings.Join(steps[0], "\n")
140+
_, stepContent := requireCopilotPreflightAndExecutionSteps(t, steps)
145141

146142
// Check that the command contains --log-level info (default)
147143
if !strings.Contains(stepContent, "--log-level info") {
@@ -166,11 +162,7 @@ func TestFirewallLogLevelInCopilotEngine(t *testing.T) {
166162
engine := NewCopilotEngine()
167163
steps := engine.GetExecutionSteps(workflowData, "test.log")
168164

169-
if len(steps) == 0 {
170-
t.Fatal("Expected at least one execution step")
171-
}
172-
173-
stepContent := strings.Join(steps[0], "\n")
165+
_, stepContent := requireCopilotPreflightAndExecutionSteps(t, steps)
174166

175167
// Check that the command contains --log-level debug
176168
if !strings.Contains(stepContent, "--log-level debug") {
@@ -198,11 +190,7 @@ func TestFirewallLogLevelInCopilotEngine(t *testing.T) {
198190
engine := NewCopilotEngine()
199191
steps := engine.GetExecutionSteps(workflowData, "test.log")
200192

201-
if len(steps) == 0 {
202-
t.Fatalf("Expected at least one execution step for log-level '%s'", level)
203-
}
204-
205-
stepContent := strings.Join(steps[0], "\n")
193+
_, stepContent := requireCopilotPreflightAndExecutionSteps(t, steps)
206194

207195
expectedFlag := "--log-level " + level
208196
if !strings.Contains(stepContent, expectedFlag) {

pkg/workflow/gh_cli_mount_test.go

Lines changed: 6 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,7 @@ func TestChrootModeInAWFContainer(t *testing.T) {
2525
engine := NewCopilotEngine()
2626
steps := engine.GetExecutionSteps(workflowData, "test.log")
2727

28-
if len(steps) == 0 {
29-
t.Fatal("Expected at least one execution step")
30-
}
31-
32-
stepContent := strings.Join(steps[0], "\n")
28+
_, stepContent := requireCopilotPreflightAndExecutionSteps(t, steps)
3329

3430
// Check that AWF is used (chroot mode is default in v0.15.0+)
3531
if !strings.Contains(stepContent, "sudo -E awf") {
@@ -53,11 +49,7 @@ func TestChrootModeInAWFContainer(t *testing.T) {
5349
engine := NewCopilotEngine()
5450
steps := engine.GetExecutionSteps(workflowData, "test.log")
5551

56-
if len(steps) == 0 {
57-
t.Fatal("Expected at least one execution step")
58-
}
59-
60-
stepContent := strings.Join(steps[0], "\n")
52+
_, stepContent := requireCopilotPreflightAndExecutionSteps(t, steps)
6153

6254
// Check that AWF command is not used
6355
if strings.Contains(stepContent, "awf") {
@@ -81,11 +73,7 @@ func TestChrootModeInAWFContainer(t *testing.T) {
8173
engine := NewCopilotEngine()
8274
steps := engine.GetExecutionSteps(workflowData, "test.log")
8375

84-
if len(steps) == 0 {
85-
t.Fatal("Expected at least one execution step")
86-
}
87-
88-
stepContent := strings.Join(steps[0], "\n")
76+
_, stepContent := requireCopilotPreflightAndExecutionSteps(t, steps)
8977

9078
// Verify AWF is present (chroot mode is default in v0.15.0+)
9179
if !strings.Contains(stepContent, "sudo -E awf") {
@@ -125,11 +113,7 @@ func TestChrootModeInAWFContainer(t *testing.T) {
125113
engine := NewCopilotEngine()
126114
steps := engine.GetExecutionSteps(workflowData, "test.log")
127115

128-
if len(steps) == 0 {
129-
t.Fatal("Expected at least one execution step")
130-
}
131-
132-
stepContent := strings.Join(steps[0], "\n")
116+
_, stepContent := requireCopilotPreflightAndExecutionSteps(t, steps)
133117

134118
// Verify AWF is present with custom args (chroot mode is default in v0.15.0+)
135119
if !strings.Contains(stepContent, "sudo -E awf") {
@@ -163,11 +147,7 @@ func TestChrootModeInAWFContainer(t *testing.T) {
163147
engine := NewCopilotEngine()
164148
steps := engine.GetExecutionSteps(workflowData, "test.log")
165149

166-
if len(steps) == 0 {
167-
t.Fatal("Expected at least one execution step")
168-
}
169-
170-
stepContent := strings.Join(steps[0], "\n")
150+
_, stepContent := requireCopilotPreflightAndExecutionSteps(t, steps)
171151

172152
// Verify AWF is being used (chroot mode is default in v0.15.0+)
173153
if !strings.Contains(stepContent, "awf") {
@@ -194,11 +174,7 @@ func TestChrootModeEnvFlags(t *testing.T) {
194174
engine := NewCopilotEngine()
195175
steps := engine.GetExecutionSteps(workflowData, "test.log")
196176

197-
if len(steps) == 0 {
198-
t.Fatal("Expected at least one execution step")
199-
}
200-
201-
stepContent := strings.Join(steps[0], "\n")
177+
_, stepContent := requireCopilotPreflightAndExecutionSteps(t, steps)
202178

203179
// Verify AWF is present (chroot mode is default in v0.15.0+)
204180
if !strings.Contains(stepContent, "sudo -E awf") {

pkg/workflow/mcp_scripts_firewall_test.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,7 @@ func TestMCPScriptsWithFirewallIncludesHostDockerInternal(t *testing.T) {
3737
engine := NewCopilotEngine()
3838
steps := engine.GetExecutionSteps(workflowData, "test.log")
3939

40-
if len(steps) == 0 {
41-
t.Fatal("Expected at least one execution step")
42-
}
43-
44-
stepContent := strings.Join(steps[0], "\n")
40+
_, stepContent := requireCopilotPreflightAndExecutionSteps(t, steps)
4541

4642
// Verify that host.docker.internal is in the allowed domains
4743
if !strings.Contains(stepContent, "host.docker.internal") {

pkg/workflow/testdata/wasm_golden/TestWasmGolden_CompileFixtures/basic-copilot.golden

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,14 @@ jobs:
328328
path: /tmp/gh-aw
329329
- name: Clean git credentials
330330
run: bash /opt/gh-aw/actions/clean_git_credentials.sh
331+
- name: Copilot pre-flight diagnostic
332+
id: copilot-preflight
333+
continue-on-error: true
334+
env:
335+
COPILOT_GITHUB_TOKEN: ${{ secrets.COPILOT_GITHUB_TOKEN }}
336+
GITHUB_SERVER_URL: ${{ github.server_url }}
337+
GITHUB_API_URL: ${{ github.api_url }}
338+
run: bash /opt/gh-aw/actions/copilot_preflight_diagnostic.sh
331339
- name: Execute GitHub Copilot CLI
332340
id: agentic_execution
333341
# Copilot CLI tool arguments (sorted):

pkg/workflow/testdata/wasm_golden/TestWasmGolden_CompileFixtures/smoke-copilot.golden

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -506,6 +506,14 @@ jobs:
506506
path: /tmp/gh-aw
507507
- name: Clean git credentials
508508
run: bash /opt/gh-aw/actions/clean_git_credentials.sh
509+
- name: Copilot pre-flight diagnostic
510+
id: copilot-preflight
511+
continue-on-error: true
512+
env:
513+
COPILOT_GITHUB_TOKEN: ${{ secrets.COPILOT_GITHUB_TOKEN }}
514+
GITHUB_SERVER_URL: ${{ github.server_url }}
515+
GITHUB_API_URL: ${{ github.api_url }}
516+
run: bash /opt/gh-aw/actions/copilot_preflight_diagnostic.sh
509517
- name: Execute GitHub Copilot CLI
510518
id: agentic_execution
511519
# Copilot CLI tool arguments (sorted):

pkg/workflow/testdata/wasm_golden/TestWasmGolden_CompileFixtures/with-imports.golden

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,14 @@ jobs:
331331
path: /tmp/gh-aw
332332
- name: Clean git credentials
333333
run: bash /opt/gh-aw/actions/clean_git_credentials.sh
334+
- name: Copilot pre-flight diagnostic
335+
id: copilot-preflight
336+
continue-on-error: true
337+
env:
338+
COPILOT_GITHUB_TOKEN: ${{ secrets.COPILOT_GITHUB_TOKEN }}
339+
GITHUB_SERVER_URL: ${{ github.server_url }}
340+
GITHUB_API_URL: ${{ github.api_url }}
341+
run: bash /opt/gh-aw/actions/copilot_preflight_diagnostic.sh
334342
- name: Execute GitHub Copilot CLI
335343
id: agentic_execution
336344
# Copilot CLI tool arguments (sorted):

0 commit comments

Comments
 (0)