test(e2e): fix debug-trace propagation wait so all 9 traffic tests run#59
Merged
Merged
Conversation
waitForDebugTraceRoute polled the gateway for a non-404 response code to
decide the new route had propagated to the data plane. That worked only
when the route's path happened to exist on the upstream too: in this
repo's setup the upstream is go-httpbin, which serves /get, /post,
/anything, etc., but not /debug-trace-test, /debug-trace-headers, etc.
Result: routes did propagate within milliseconds, the gateway matched
them, but it forwarded e.g. /debug-trace-test to httpbin which replied
with its own 404. The wait loop interpreted that as "not propagated
yet", spent the full 15s timeout, then soft-skipped.
This was masked because some debug-trace tests already set their own
proxy-rewrite plugin (rewriting to /post, /anything/*) for unrelated
reasons; those tests passed in roughly half a second. The four tests
that did NOT supply a plugin (JSONOutput, WithHeaders, WithHost,
YAMLOutput) always skipped.
Inject a default `proxy-rewrite: { uri: "/anything" }` in
createDebugTraceRoute when the caller does not supply its own plugins.
go-httpbin's /anything endpoint always returns 200 with a JSON echo, so
the wait loop sees the route the moment it lands at the gateway.
Callers that DO supply plugins (WithMethod, WithPath, WithHost) are
unaffected.
Local rerun: all 9 traffic tests pass; total wall time dropped from
67s to 8s.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough
ChangesDebug Trace Route Enhancement
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the e2e debug-trace route helper so route propagation waiting succeeds even when synthetic test paths aren’t served by the upstream (go-httpbin), preventing unintended 15s timeouts and soft-skips.
Changes:
- Injects a default
proxy-rewriteplugin (to/anything) when creating debug-trace routes without caller-supplied plugins. - Adds
stringsimport to support the plugin-presence guard.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
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
`waitForDebugTraceRoute` polled the gateway for a non-404 response code to decide a newly-created route had propagated to the data plane. That signal only works when the route's path happens to exist on the upstream too. In this repo's setup the upstream is `go-httpbin`, which serves `/get`, `/post`, `/anything`, etc., but not `/debug-trace-test`, `/debug-trace-headers`, `/debug-trace-yaml`, etc.
So routes did propagate within milliseconds, the gateway matched them, but it forwarded e.g. `/debug-trace-test` to httpbin, which replied with its own 404. The wait loop interpreted that as "not propagated yet", spent the full 15s timeout, then soft-skipped.
Reproduction
```bash
A7_GATEWAY_URL=http://localhost:9080 HTTPBIN_URL=http://host.docker.internal:8081 \
go test -tags e2e -v -count=1 -run TestDebugTrace_JSONOutput ./test/e2e/...
```
Before this PR:
```
--- SKIP: TestDebugTrace_JSONOutput (15.35s)
debug_test.go:59: route /debug-trace-test did not propagate to the local gateway within timeout
```
A direct curl during that 15s window returns a 404 with `Server: API7/3.9.12` and `Content-Type: text/plain` — the signature of go-httpbin's not-found handler, NOT the gateway's own `{"error_msg":"404 Route Not Found"}`. The route IS at the gateway; the upstream just doesn't serve the synthetic path.
Fix
In `createDebugTraceRoute`, when the caller does not supply its own plugins, inject a default `proxy-rewrite: { uri: "/anything" }`. `/anything` is a path go-httpbin always returns 200 for (with a JSON echo of the request), so the wait loop sees the route the moment the data plane picks it up. Callers that DO supply plugins (`WithMethod`, `WithPath`, `WithHost`) are unaffected — the existing `strings.Contains(extraFields, "plugins")` guard keeps their custom plugin set intact.
Test plan
Local rerun against API7 EE 3.9.12 with go-httpbin upstream:
9/9 PASS, 0/9 skip. Total wall time dropped from 67s to 8s.
Summary by CodeRabbit