test(serve): stabilize test_reload_yaml_error flake (status==0 → retry)#331
Open
avrabe wants to merge 1 commit into
Open
test(serve): stabilize test_reload_yaml_error flake (status==0 → retry)#331avrabe wants to merge 1 commit into
avrabe wants to merge 1 commit into
Conversation
📐 Rivet artifact deltaNo artifact changes in this PR. Code-only changes (renderer, CLI wiring, tests) don't touch the artifact graph. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark 'Rivet Criterion Benchmarks'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.
| Benchmark suite | Current: 7c24058 | Previous: fce5dff | Ratio |
|---|---|---|---|
link_graph_build/10000 |
35759577 ns/iter (± 2730637) |
27552219 ns/iter (± 1885336) |
1.30 |
validate/10000 |
22973605 ns/iter (± 2730246) |
13419892 ns/iter (± 138543) |
1.71 |
This comment was automatically generated by workflow using github-action-benchmark.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
avrabe
force-pushed
the
fix/test-reload-yaml-flake
branch
4 times, most recently
from
0d3c632 to
6714248
Compare
Same transient-connection-drop flake class that landed in fetch_page_with_retry (PR-train preceding v0.13.0). The raw TcpStream POST + read_to_end occasionally returns empty data on self-hosted-runner contention, leaving the HTTP status parsed as 0 and the assertion failing with "got status 0" — observed on PR #328 (no serve-code change in the PR, the failure was the test, not the production code). Extract a `post_reload_status` helper that retries once on `status == 0` after a 200 ms backoff (mirroring fetch_page_with_retry exactly), and use it from test_reload_yaml_error_returns_error_response. The other reload tests already work fine via fetch_with_timeout so they don't need the helper yet. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
avrabe
force-pushed
the
fix/test-reload-yaml-flake
branch
from
6714248 to
7c24058
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Stabilizes `test_reload_yaml_error_returns_error_response` against the same transient-connection-drop flake class `fetch_page_with_retry` already handles for GETs (observed in PR #328's CI run, which doesn't touch any serve code — the failure was the test, not the production code).
The raw `TcpStream` POST + `read_to_end` occasionally returns empty data on self-hosted-runner contention, leaving the HTTP status parsed as `0` and the assertion failing with `got status 0`. Extract a `post_reload_status` helper that retries once on `status == 0` after a 200 ms backoff — mirrors `fetch_page_with_retry` exactly.
Test plan
🤖 Generated with Claude Code