Skip to content

Commit 7d86829

Browse files
Reduce merge queue flakiness (#2484)
* ci: reduce merge queue flakiness Contain hanging .NET test processes while preserving diagnostics, make Java native artifacts stable across rerun attempts, retry transient package downloads, and allow slower Rust proxy startup on loaded runners. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(test): prevent replay proxy teardown hangs Force-close active proxy connections and upstream requests during shutdown so fixture disposal cannot wait forever after a streaming test. Cover pending requests and streaming responses with regression tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * ci: preserve existing .NET timeout budgets Keep the existing job and hang-detection timeouts while retaining diagnostic output for genuine failures. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test: run harness regression tests in CI Execute the harness suite once in the Node workflow and include it in the repository-wide test target so proxy shutdown coverage gates changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test: isolate replay writes from CI environment Replay proxy unit tests verify snapshot serialization, so preserve and clear the ambient GITHUB_ACTIONS flag around each test instead of suppressing their output files in CI. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent c1a7d5b commit 7d86829

9 files changed

Lines changed: 151 additions & 19 deletions

File tree

‎.github/workflows/dotnet-sdk-tests.yml‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,16 @@ jobs:
175175
# whole suite normally finishes in about five.
176176
# The validation job performs the full analyzer-enabled SDK build.
177177
# Build only test-consumed frameworks here and do not repeat analyzers.
178-
args=(--no-restore -v n --blame-hang --blame-hang-timeout 10m --blame-hang-dump-type none -p:RunAnalyzers=false)
178+
args=(
179+
--no-restore
180+
-v n
181+
--blame-hang
182+
--blame-hang-timeout 10m
183+
--blame-hang-dump-type none
184+
--logger "trx;LogFilePrefix=test-results"
185+
--results-directory "$GITHUB_WORKSPACE/dotnet/TestResults"
186+
-p:RunAnalyzers=false
187+
)
179188
180189
filter="$DOTNET_TEST_FILTER"
181190
if [[ "$DOTNET_TEST_SHARD" != "full" ]]; then
@@ -217,3 +226,12 @@ jobs:
217226
args+=(--filter "$filter")
218227
fi
219228
dotnet test test/GitHub.Copilot.SDK.Test.csproj "${args[@]}"
229+
230+
- name: Upload .NET test diagnostics
231+
if: failure()
232+
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7
233+
with:
234+
name: dotnet-test-diagnostics-${{ matrix.os }}-${{ matrix.transport }}-${{ matrix.backend }}-${{ matrix.shard }}-${{ github.run_attempt }}
235+
path: dotnet/TestResults/
236+
if-no-files-found: warn
237+
retention-days: 7

‎.github/workflows/java-sdk-tests.yml‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,11 @@ on:
77
permissions:
88
contents: read
99

10+
env:
11+
MAVEN_OPTS: >-
12+
-Daether.connector.http.retryHandler.count=3
13+
-Daether.connector.http.retryHandler.serviceUnavailable=429,502,503
14+
1015
jobs:
1116
java-sdk-inprocess:
1217
name: "Java SDK InProcess Tests (${{ matrix.classifier }})"
@@ -121,11 +126,12 @@ jobs:
121126
122127
- uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7
123128
with:
124-
name: java-native-publication-linux-arm64-${{ github.run_id }}-${{ github.run_attempt }}
129+
name: java-native-publication-linux-arm64-${{ github.run_id }}
125130
path: |
126131
java/copilot-native/target/copilot-sdk-java-runtime-${{ steps.build.outputs.version }}-linux-arm64.jar
127132
java/copilot-native/target/linux-arm64-${{ steps.build.outputs.version }}.sha256
128133
if-no-files-found: error
134+
overwrite: true
129135
retention-days: 1
130136

131137
java-native-publication-windows:
@@ -178,11 +184,12 @@ jobs:
178184
179185
- uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7
180186
with:
181-
name: java-native-publication-win32-x64-${{ github.run_id }}-${{ github.run_attempt }}
187+
name: java-native-publication-win32-x64-${{ github.run_id }}
182188
path: |
183189
java/copilot-native/target/copilot-sdk-java-runtime-${{ steps.build.outputs.version }}-win32-x64.jar
184190
java/copilot-native/target/win32-x64-${{ steps.build.outputs.version }}.sha256
185191
if-no-files-found: error
192+
overwrite: true
186193
retention-days: 1
187194

188195
java-native-publication-windows-arm64:
@@ -235,11 +242,12 @@ jobs:
235242
236243
- uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7
237244
with:
238-
name: java-native-publication-win32-arm64-${{ github.run_id }}-${{ github.run_attempt }}
245+
name: java-native-publication-win32-arm64-${{ github.run_id }}
239246
path: |
240247
java/copilot-native/target/copilot-sdk-java-runtime-${{ steps.build.outputs.version }}-win32-arm64.jar
241248
java/copilot-native/target/win32-arm64-${{ steps.build.outputs.version }}.sha256
242249
if-no-files-found: error
250+
overwrite: true
243251
retention-days: 1
244252

245253
java-native-publication-darwin:
@@ -293,11 +301,12 @@ jobs:
293301
294302
- uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7
295303
with:
296-
name: java-native-publication-darwin-arm64-${{ github.run_id }}-${{ github.run_attempt }}
304+
name: java-native-publication-darwin-arm64-${{ github.run_id }}
297305
path: |
298306
java/copilot-native/target/copilot-sdk-java-runtime-${{ steps.build.outputs.version }}-darwin-arm64.jar
299307
java/copilot-native/target/darwin-arm64-${{ steps.build.outputs.version }}.sha256
300308
if-no-files-found: error
309+
overwrite: true
301310
retention-days: 1
302311

303312
java-native-publication-assembly:
@@ -333,22 +342,22 @@ jobs:
333342

334343
- uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4.3.0
335344
with:
336-
name: java-native-publication-linux-arm64-${{ github.run_id }}-${{ github.run_attempt }}
345+
name: java-native-publication-linux-arm64-${{ github.run_id }}
337346
path: ${{ github.workspace }}/java/native-publication-input/linux-arm64
338347

339348
- uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4.3.0
340349
with:
341-
name: java-native-publication-win32-x64-${{ github.run_id }}-${{ github.run_attempt }}
350+
name: java-native-publication-win32-x64-${{ github.run_id }}
342351
path: ${{ github.workspace }}/java/native-publication-input/windows
343352

344353
- uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4.3.0
345354
with:
346-
name: java-native-publication-win32-arm64-${{ github.run_id }}-${{ github.run_attempt }}
355+
name: java-native-publication-win32-arm64-${{ github.run_id }}
347356
path: ${{ github.workspace }}/java/native-publication-input/windows-arm64
348357

349358
- uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4.3.0
350359
with:
351-
name: java-native-publication-darwin-arm64-${{ github.run_id }}-${{ github.run_attempt }}
360+
name: java-native-publication-darwin-arm64-${{ github.run_id }}
352361
path: ${{ github.workspace }}/java/native-publication-input/darwin
353362

354363
- name: Verify native inputs and deploy the complete local release

‎.github/workflows/nodejs-sdk-tests.yml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,11 @@ jobs:
5353
working-directory: ./test/harness
5454
run: npm ci --ignore-scripts
5555

56+
- name: Run test harness tests
57+
if: runner.os == 'Linux' && matrix.transport == 'default'
58+
working-directory: ./test/harness
59+
run: npm test
60+
5661
- name: Warm up PowerShell
5762
if: runner.os == 'Windows'
5863
run: pwsh.exe -Command "Write-Host 'PowerShell ready'"

‎.github/workflows/python-sdk-tests.yml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ jobs:
5050

5151
- name: Install Node.js dependencies (for CLI in tests)
5252
working-directory: ./nodejs
53-
run: npm ci --ignore-scripts
53+
run: npm ci --ignore-scripts --fetch-retries=4 --fetch-retry-mintimeout=10000 --fetch-retry-maxtimeout=60000
5454

5555
- name: Run ruff format check
5656
run: uv run ruff format --check .
@@ -63,7 +63,7 @@ jobs:
6363

6464
- name: Install test harness dependencies
6565
working-directory: ./test/harness
66-
run: npm ci --ignore-scripts
66+
run: npm ci --ignore-scripts --fetch-retries=4 --fetch-retry-mintimeout=10000 --fetch-retry-maxtimeout=60000
6767

6868
- name: Warm up PowerShell
6969
if: runner.os == 'Windows'

‎justfile‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ format: format-go format-python format-nodejs format-dotnet format-rust
99
lint: lint-go lint-python lint-nodejs lint-dotnet lint-rust
1010

1111
# Run tests for all languages
12-
test: test-go test-python test-nodejs test-dotnet test-rust test-corrections
12+
test: test-go test-python test-nodejs test-dotnet test-rust test-harness test-corrections
1313

1414
# Format Go code
1515
format-go:
@@ -66,6 +66,11 @@ test-nodejs:
6666
@echo "=== Testing Node.js code ==="
6767
@cd nodejs && npm test
6868

69+
# Run test harness tests
70+
test-harness:
71+
@echo "=== Testing test harness ==="
72+
@cd test/harness && npm test
73+
6974
# Test .NET code
7075
test-dotnet:
7176
@echo "=== Testing .NET code ==="
@@ -168,4 +173,3 @@ validate-docs-go:
168173
validate-docs-cs:
169174
@echo "=== Validating C# documentation ==="
170175
@cd scripts/docs-validation && npm run validate:cs
171-

‎rust/tests/e2e/support.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ static SHARED_E2E_RUNTIME: LazyLock<tokio::runtime::Runtime> = LazyLock::new(||
3131
.expect("create shared E2E runtime")
3232
});
3333
const SHARED_E2E_CLEANUP_TIMEOUT: Duration = Duration::from_secs(10);
34+
const PROXY_STARTUP_TIMEOUT: Duration = Duration::from_secs(30);
3435

3536
pub const DEFAULT_TEST_TOKEN: &str = "rust-e2e-token";
3637

@@ -1274,7 +1275,7 @@ impl CapiProxy {
12741275
}
12751276
});
12761277
let re = regex::Regex::new(r"Listening: (http://[^\s]+)\s+(\{.*\})$").unwrap();
1277-
let deadline = Instant::now() + SHARED_E2E_CLEANUP_TIMEOUT;
1278+
let deadline = Instant::now() + PROXY_STARTUP_TIMEOUT;
12781279
while let Some(remaining) = deadline.checked_duration_since(Instant::now()) {
12791280
let line = match line_rx.recv_timeout(remaining) {
12801281
Ok(Ok(line)) => line,
@@ -1341,7 +1342,7 @@ impl CapiProxy {
13411342

13421343
kill_and_wait_child(&mut child);
13431344
Err(std::io::Error::other(format!(
1344-
"timed out after {SHARED_E2E_CLEANUP_TIMEOUT:?} waiting for proxy startup"
1345+
"timed out after {PROXY_STARTUP_TIMEOUT:?} waiting for proxy startup"
13451346
)))
13461347
}
13471348

‎test/harness/capturingHttpProxy.test.ts‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,21 @@ describe("Capturing HTTP Proxy", () => {
1010
let proxy: CapturingHttpProxy;
1111
let testServer: http.Server;
1212
let testServerAddress: string;
13+
let onHangingRequest: (() => void) | undefined;
14+
let onStreamingResponse: (() => void) | undefined;
1315

1416
beforeEach(async () => {
1517
testServer = http.createServer((req, res) => {
18+
if (req.url === "/hang") {
19+
onHangingRequest?.();
20+
return;
21+
}
22+
if (req.url === "/stream") {
23+
res.writeHead(200, { "content-type": "text/plain" });
24+
res.write("started");
25+
onStreamingResponse?.();
26+
return;
27+
}
1628
res.writeHead(200, { "content-type": "application/json" });
1729
res.end(JSON.stringify({ message: "Hello", path: req.url }));
1830
});
@@ -71,4 +83,34 @@ describe("Capturing HTTP Proxy", () => {
7183
} as CapturedExchange,
7284
]);
7385
});
86+
87+
test("stops while a proxied request is still active", async () => {
88+
proxy = new CapturingHttpProxy(testServerAddress);
89+
const proxyUrl = await proxy.start();
90+
const requestStarted = new Promise<void>((resolve) => {
91+
onHangingRequest = resolve;
92+
});
93+
const request = fetch(`${proxyUrl}/hang`).catch(() => undefined);
94+
await requestStarted;
95+
96+
await proxy.stop();
97+
98+
await request;
99+
});
100+
101+
test("stops while a proxied response is still streaming", async () => {
102+
proxy = new CapturingHttpProxy(testServerAddress);
103+
const proxyUrl = await proxy.start();
104+
const responseStarted = new Promise<void>((resolve) => {
105+
onStreamingResponse = resolve;
106+
});
107+
const responsePromise = fetch(`${proxyUrl}/stream`);
108+
await responseStarted;
109+
const response = await responsePromise;
110+
const body = response.text().catch(() => undefined);
111+
112+
await proxy.stop();
113+
114+
await body;
115+
});
74116
});

‎test/harness/capturingHttpProxy.ts‎

Lines changed: 49 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,10 @@ import https from "https";
1010
*/
1111
export class CapturingHttpProxy {
1212
private readonly capturedExchanges: CapturedExchange[] = [];
13+
private readonly activeRequests = new Set<http.ClientRequest>();
14+
private readonly activeResponses = new Set<http.IncomingMessage>();
1315
private server?: http.Server;
16+
private stopPromise?: Promise<void>;
1417

1518
constructor(private targetUrl: string) {}
1619

@@ -90,6 +93,10 @@ export class CapturingHttpProxy {
9093
res.end();
9194
},
9295
onError: (err) => {
96+
if (!this.server) {
97+
res.destroy();
98+
return;
99+
}
93100
console.error("Error in proxying request:", err);
94101
const endTime = Date.now();
95102
const formattedError =
@@ -130,24 +137,58 @@ export class CapturingHttpProxy {
130137
}
131138

132139
async stop(): Promise<void> {
133-
if (this.server) {
134-
return new Promise((resolve, reject) => {
135-
this.server!.close((err) => {
140+
if (this.stopPromise) {
141+
return this.stopPromise;
142+
}
143+
144+
const server = this.server;
145+
if (!server) {
146+
return;
147+
}
148+
149+
this.server = undefined;
150+
this.stopPromise = (async () => {
151+
const closed = new Promise<void>((resolve, reject) => {
152+
server.close((err) => {
136153
if (err) {
137154
reject(err);
138155
} else {
139156
resolve();
140157
}
141158
});
142159
});
143-
}
160+
161+
// server.close() waits for active connections. A replayed streaming request
162+
// can otherwise wedge fixture teardown after its test has already passed.
163+
server.closeAllConnections();
164+
for (const response of this.activeResponses) {
165+
response.destroy();
166+
}
167+
this.activeResponses.clear();
168+
for (const request of this.activeRequests) {
169+
request.destroy();
170+
}
171+
this.activeRequests.clear();
172+
173+
await closed;
174+
})();
175+
return this.stopPromise;
144176
}
145177

146178
performRequest(options: PerformRequestOptions): void {
179+
if (this.stopPromise) {
180+
options.onError(new Error("Proxy is stopping"));
181+
return;
182+
}
183+
147184
const protocol = options.isHttps ? https : http;
148185
const upstreamRequest = protocol.request(
149186
options.requestOptions,
150187
(upstreamResponse) => {
188+
this.activeResponses.add(upstreamResponse);
189+
upstreamResponse.once("close", () => {
190+
this.activeResponses.delete(upstreamResponse);
191+
});
151192
options.onResponseStart(
152193
upstreamResponse.statusCode || 500,
153194
upstreamResponse.headers,
@@ -157,6 +198,10 @@ export class CapturingHttpProxy {
157198
},
158199
);
159200

201+
this.activeRequests.add(upstreamRequest);
202+
upstreamRequest.once("close", () => {
203+
this.activeRequests.delete(upstreamRequest);
204+
});
160205
upstreamRequest.on("error", options.onError);
161206

162207
if (options.body) {

‎test/harness/replayingCapiProxy.test.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,21 @@ import { ShellConfig } from "./util";
2424
describe("ReplayingCapiProxy", () => {
2525
let tempDir: string;
2626
let workDir: string;
27+
let githubActions: string | undefined;
2728

2829
beforeEach(async () => {
30+
githubActions = process.env.GITHUB_ACTIONS;
31+
delete process.env.GITHUB_ACTIONS;
2932
tempDir = await mkdtemp(path.join(os.tmpdir(), "capi-proxy-test-"));
3033
workDir = path.join(tempDir, "work");
3134
});
3235

3336
afterEach(async () => {
37+
if (githubActions === undefined) {
38+
delete process.env.GITHUB_ACTIONS;
39+
} else {
40+
process.env.GITHUB_ACTIONS = githubActions;
41+
}
3442
await rm(tempDir, { recursive: true, force: true });
3543
});
3644

0 commit comments

Comments
 (0)