Skip to content

test: strengthen E2E readback assertions#6

Merged
guoqqqi merged 4 commits intomasterfrom
codex/e2e-control-plane-readback
Apr 28, 2026
Merged

test: strengthen E2E readback assertions#6
guoqqqi merged 4 commits intomasterfrom
codex/e2e-control-plane-readback

Conversation

@guoqqqi
Copy link
Copy Markdown
Contributor

@guoqqqi guoqqqi commented Apr 28, 2026

Summary

  • Add a shared E2E helper for running a7 commands and decoding JSON output.
  • Strengthen control-plane E2E coverage to verify resources through a7 ... get -o json after create/update operations.
  • Add delete readback checks for core resources so CRUD tests verify cleanup behavior.
  • Extend config sync coverage by dumping the remote config after sync and asserting the synced service/route are present.

Scope

This keeps the current CI model: GitHub Actions uses repository-configured API7 environment variables. It does not add a mandatory gateway/data-plane dependency; gateway traffic tests remain optional behind A7_GATEWAY_URL / HTTPBIN_URL.

Refs #1

Validation

  • go test ./...
  • go test -c -tags=e2e -o /tmp/a7-e2e.test ./test/e2e
  • make validate-skills
  • make test-skills
  • git diff --check

Note: make test-e2e was attempted locally but stopped because this machine does not have A7_ADMIN_URL configured; the PR CI should run it with repository variables.

Summary by CodeRabbit

  • Tests
    • Enhanced e2e test validation across multiple resource types to use structured JSON parsing for more robust assertions on API responses, replacing text-based substring matching.
    • Strengthened deletion verification by explicitly confirming operations fail after resource removal.
    • Improved test reliability and consistency in validating resource properties and configurations.

Copilot AI review requested due to automatic review settings April 28, 2026 08:43
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 28, 2026

Warning

Rate limit exceeded

@guoqqqi has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 2 minutes and 29 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9d544b56-0598-4d11-acbe-44e545938735

📥 Commits

Reviewing files that changed from the base of the PR and between 2982b63 and ec1998a.

📒 Files selected for processing (12)
  • test/e2e/config_sync_test.go
  • test/e2e/credential_test.go
  • test/e2e/global_rule_test.go
  • test/e2e/plugin_metadata_test.go
  • test/e2e/proto_test.go
  • test/e2e/route_test.go
  • test/e2e/secret_test.go
  • test/e2e/service_template_test.go
  • test/e2e/service_test.go
  • test/e2e/setup_test.go
  • test/e2e/ssl_test.go
  • test/e2e/stream_route_test.go
📝 Walkthrough

Walkthrough

A new E2E helper function runA7JSON is introduced to parse CLI JSON output into structured Go types. All E2E CRUD tests are refactored to replace stdout substring validation with field-level assertions using the new helper, and deletion verification is strengthened with explicit post-delete get commands.

Changes

Cohort / File(s) Summary
New E2E Helper
test/e2e/setup_test.go
Introduces runA7JSON function that wraps runA7WithEnv to decode JSON command output into caller-provided structures, validating both execution and unmarshalling success.
CRUD Test Refactoring: Structured JSON Validation
test/e2e/config_sync_test.go, test/e2e/consumer_test.go, test/e2e/credential_test.go, test/e2e/global_rule_test.go, test/e2e/plugin_metadata_test.go, test/e2e/proto_test.go, test/e2e/route_test.go, test/e2e/secret_test.go, test/e2e/service_template_test.go, test/e2e/service_test.go, test/e2e/ssl_test.go, test/e2e/stream_route_test.go
Consistently refactor JSON validation from stdout substring checks to parsed field assertions via runA7JSON, add explicit post-delete error verification, and strengthen update verification with JSON readback in applicable tests. Route test removes unused encoding/json import.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • test: migrate runtime coverage toward ginkgo e2e #3: Both PRs involve E2E test helper modifications and CLI JSON handling—this PR adds the runA7JSON helper and applies it across all CRUD tests, extending prior work on E2E test infrastructure and JSON output handling.

Suggested reviewers

  • Copilot

Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error)

Check name Status Explanation Resolution
Security Check ❌ Error Pull request contains three instances of sensitive data exposure in test assertion failures by logging raw stdout and stderr containing API payloads, credentials, and plugin configurations to CI logs. Remove raw stdout and stderr from require.NoError messages in setup_test.go lines 185-186, credential_test.go lines 90-93, and secret_test.go lines 93-94. Replace with context-only messages that do not include command output containing sensitive data.
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title accurately summarizes the main change: strengthening E2E readback assertions across multiple test files by adding JSON parsing and verification checks.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
E2e Test Quality Review ✅ Passed PR successfully strengthens E2E test quality by introducing runA7JSON helper for structured JSON assertions and enhancing all CRUD tests with explicit readback assertions after create/update operations and deletion verification.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/e2e-control-plane-readback

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR strengthens the E2E suite by shifting “readback” assertions from stdout string-matching to structured JSON decoding, and by adding post-delete checks so CRUD tests validate cleanup behavior.

Changes:

  • Added a shared runA7JSON E2E helper to run a7 commands and unmarshal -o json output.
  • Updated multiple CRUD-style E2E tests to assert specific JSON fields after create/update operations.
  • Added post-delete “get should fail” checks and extended config sync coverage by dumping remote config after sync.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
test/e2e/setup_test.go Adds runA7JSON helper to execute CLI and decode JSON stdout.
test/e2e/route_test.go Strengthens route CRUD/create assertions using JSON readbacks and adds delete readback.
test/e2e/service_test.go Strengthens service CRUD/export/plugin assertions using JSON readbacks and adds delete readback.
test/e2e/consumer_test.go Strengthens consumer CRUD assertions using JSON readbacks and adds delete readback.
test/e2e/credential_test.go Adds JSON readback assertions and update+readback coverage for credentials, plus delete readback.
test/e2e/ssl_test.go Replaces stdout matching with JSON assertions and adds delete readback.
test/e2e/secret_test.go Replaces stdout matching with JSON assertions, adds update+readback and delete readback.
test/e2e/stream_route_test.go Replaces stdout matching with JSON assertions, adds update+readback and delete readback.
test/e2e/proto_test.go Replaces stdout matching with JSON assertions and adds delete readback.
test/e2e/plugin_metadata_test.go Replaces stdout matching with JSON assertions and adds delete readback.
test/e2e/global_rule_test.go Replaces stdout matching with JSON assertions, improves skill-example verification, adds delete readback.
test/e2e/service_template_test.go Replaces stdout matching with JSON assertions and adds delete readback.
test/e2e/config_sync_test.go Uses runA7JSON for route readback and adds post-sync remote config dump assertions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread test/e2e/setup_test.go
Comment on lines +184 to +189
func runA7JSON(t testTB, env []string, out interface{}, args ...string) string {
t.Helper()
stdout, stderr, err := runA7WithEnv(env, args...)
require.NoError(t, err, "stdout=%s stderr=%s", stdout, stderr)
require.NoError(t, json.Unmarshal([]byte(stdout), out), "stdout=%s", stdout)
return stdout
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (7)
test/e2e/config_sync_test.go (1)

185-186: Optional: validate dump content structurally instead of substring matching

assert.Contains on raw YAML can produce false positives. Parsing and checking services/routes IDs directly would make this check stricter.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/e2e/config_sync_test.go` around lines 185 - 186, The test currently uses
assert.Contains on the raw YAML dump (variables dumped, routeID, svcID) which
can yield false positives; instead parse the dumped YAML (e.g., yaml.Unmarshal
into a map[string]interface{} or a small struct) and then assert the presence of
svcID under the "services" keys and routeID under the "routes" entries. Update
the assertions in config_sync_test.go to unmarshal dumped into a parsed
structure and use direct existence checks (lookup map keys or iterate parsed
slices) for svcID and routeID rather than substring matching.
test/e2e/service_test.go (1)

160-160: Make plugin assertion structural

At Line 160, checking fmt.Sprint(service["plugins"]) is brittle. Prefer asserting the "proxy-rewrite" key on the decoded plugins object.

♻️ Suggested refactor
 	assert.Equal(t, svcID, service["id"])
-	assert.Contains(t, fmt.Sprint(service["plugins"]), "proxy-rewrite")
+	plugins, ok := service["plugins"].(map[string]interface{})
+	require.True(t, ok, "plugins should be an object")
+	_, exists := plugins["proxy-rewrite"]
+	assert.True(t, exists, "proxy-rewrite plugin should exist")
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/e2e/service_test.go` at line 160, The test currently asserts on a
stringified plugins value (fmt.Sprint(service["plugins"])) which is brittle;
instead decode service["plugins"] into its concrete structure (e.g.
map[string]interface{} or []interface{}) and assert the presence of the
"proxy-rewrite" key/entry directly. Locate the assertion using
service["plugins"] in the test (around the assert.Contains call) and replace it
with a structural check that verifies the key exists (e.g., type-assert to map
and check map["proxy-rewrite"] or iterate a slice and match the plugin name) so
the test is robust against formatting changes.
test/e2e/ssl_test.go (1)

83-83: Tighten snis assertion to typed list checks

At Line 83 and Line 87, fmt.Sprint(ssl["snis"]) is weaker than asserting membership on the decoded array.

♻️ Suggested refactor
 	runA7JSON(t, env, &ssl, "ssl", "get", sslID, "-g", gatewayGroup, "-o", "json")
 	assert.Equal(t, sslID, ssl["id"])
-	assert.Contains(t, fmt.Sprint(ssl["snis"]), "e2e-test.example.com")
+	snis, ok := ssl["snis"].([]interface{})
+	require.True(t, ok, "snis should be an array")
+	assert.Contains(t, snis, "e2e-test.example.com")
@@
 	runA7JSON(t, env, &ssl, "ssl", "get", sslID, "-g", gatewayGroup, "-o", "json")
-	assert.Contains(t, fmt.Sprint(ssl["snis"]), "e2e-test.example.com")
+	snis, ok = ssl["snis"].([]interface{})
+	require.True(t, ok, "snis should be an array")
+	assert.Contains(t, snis, "e2e-test.example.com")

Also applies to: 87-87

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/e2e/ssl_test.go` at line 83, The test currently converts ssl["snis"] to
a string via fmt.Sprint and asserts substring membership; instead decode the
value as a typed slice and assert membership on that slice. Locate the test
variable ssl and replace the two assertions that use fmt.Sprint(ssl["snis"])
with code that type-asserts ssl["snis"] to []interface{} (or []string),
map/convert it to []string (e.g., iterate and cast each element), and then use
assert.Contains(t, convertedSlice, "e2e-test.example.com") (or assert.True with
a simple loop membership check) for both occurrences so the test checks the
actual array contents rather than stringified output.
test/e2e/plugin_metadata_test.go (1)

57-60: Prefer structural assertions over stringified map checks

At Line 59 and Line 60, fmt.Sprint(metadata) checks can pass on incidental text. Assert the nested fields directly for deterministic coverage.

♻️ Suggested refactor
 	var metadata map[string]interface{}
 	runA7JSON(t, env, &metadata, "plugin-metadata", "get", pluginName, "-g", gatewayGroup, "-o", "json")
-	assert.Contains(t, fmt.Sprint(metadata), "log_format")
-	assert.Contains(t, fmt.Sprint(metadata), "client_ip")
+	logFormat, ok := metadata["log_format"].(map[string]interface{})
+	require.True(t, ok, "log_format should be an object")
+	assert.Equal(t, "$remote_addr", logFormat["client_ip"])
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/e2e/plugin_metadata_test.go` around lines 57 - 60, Replace the brittle
stringified map checks that use fmt.Sprint(metadata) with direct structural
assertions against the parsed map returned by runA7JSON; locate the test
variables (metadata) and the call runA7JSON(t, env, &metadata,
"plugin-metadata", "get", pluginName, "-g", gatewayGroup, "-o", "json") and
assert the presence of keys and nested fields by performing safe type assertions
(e.g. check metadata is a map[string]interface{}, then check nested map keys
like "log_format" and "client_ip" exist and are non-nil/expected) using
assert.NotNil/assert.True/assert.Contains or assert.Equal as appropriate rather
than relying on fmt.Sprint.
test/e2e/credential_test.go (1)

72-77: Use structural plugin assertions instead of stringified map matching.

At Line 76 and Line 97, fmt.Sprint(credential["plugins"]) + substring matching is brittle. Assert plugin-map membership directly to keep this test truly schema-level.

♻️ Proposed change
-	assert.Contains(t, fmt.Sprint(credential["plugins"]), "key-auth")
+	plugins, ok := credential["plugins"].(map[string]interface{})
+	require.True(t, ok, "plugins should be a JSON object")
+	_, ok = plugins["key-auth"]
+	assert.True(t, ok, "expected key-auth plugin in credential readback")
...
-	assert.Contains(t, fmt.Sprint(credential["plugins"]), "key-auth")
+	plugins, ok = credential["plugins"].(map[string]interface{})
+	require.True(t, ok, "plugins should be a JSON object")
+	_, ok = plugins["key-auth"]
+	assert.True(t, ok, "expected key-auth plugin after update")

Also applies to: 94-97

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/e2e/credential_test.go` around lines 72 - 77, The test currently checks
plugins by doing fmt.Sprint(credential["plugins"]) and substring matching which
is brittle; instead, after calling runA7JSON and populating the credential map,
assert the plugin structure directly by casting credential["plugins"] to the
expected collection type (e.g., []interface{} or map[string]interface{}),
iterate its entries and assert that one entry has the plugin identifier (e.g.,
plugin["name"] == "key-auth" or plugin["id"] matching expected) using
assert.True/assert.Equal; update both occurrences around the
runA7JSON/credential usage so you validate plugin-map membership structurally
rather than via string matching.
test/e2e/global_rule_test.go (1)

66-70: Prefer map-key assertions over fmt.Sprint substring checks for plugins.

These checks still validate rendered text, not decoded structure. Since this PR’s goal is stronger readback assertions, directly assert plugin keys from map[string]interface{}.

♻️ Proposed change pattern
-	assert.Contains(t, fmt.Sprint(globalRule["plugins"]), "prometheus")
+	plugins, ok := globalRule["plugins"].(map[string]interface{})
+	require.True(t, ok, "plugins should be a JSON object")
+	_, ok = plugins["prometheus"]
+	assert.True(t, ok)
...
-	assert.Contains(t, fmt.Sprint(globalRule["plugins"]), "prefer_name")
+	prom, ok := plugins["prometheus"].(map[string]interface{})
+	require.True(t, ok, "prometheus plugin should be an object")
+	assert.Equal(t, true, prom["prefer_name"])

Also applies to: 85-86, 126-130

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/e2e/global_rule_test.go` around lines 66 - 70, The test currently checks
plugins via fmt.Sprint(globalRule["plugins"]) which inspects rendered text;
instead, type-assert globalRule["plugins"] to its decoded form (e.g.,
map[string]interface{} or []interface{}) and assert the presence of the
"prometheus" plugin key/value directly (for a map: assert.Contains(t,
pluginsMap, "prometheus"); for a slice: iterate and check item names). Update
the check in the globalRule assertion block (variable globalRule and call
runA7JSON) and apply the same change to the other occurrences mentioned (the
checks around lines 85-86 and 126-130) so assertions operate on the decoded
structure rather than fmt.Sprint output.
test/e2e/route_test.go (1)

113-117: Convert remaining string-based checks to typed JSON assertions.

At Line 116/158-160/196, fmt.Sprint(...) assertions can pass for the wrong reasons. Assert slice/map content directly for paths, methods, labels, and plugins.

♻️ Proposed assertion style
-	assert.Contains(t, fmt.Sprint(route["paths"]), "/test-updated")
+	paths, ok := route["paths"].([]interface{})
+	require.True(t, ok, "paths should be an array")
+	assert.Contains(t, paths, "/test-updated")

-	assert.Contains(t, fmt.Sprint(route["methods"]), "GET")
+	methods, ok := route["methods"].([]interface{})
+	require.True(t, ok, "methods should be an array")
+	assert.Contains(t, methods, "GET")

-	assert.Contains(t, fmt.Sprint(route["labels"]), "env")
-	assert.Contains(t, fmt.Sprint(route["labels"]), "team")
+	labels, ok := route["labels"].(map[string]interface{})
+	require.True(t, ok, "labels should be an object")
+	assert.Equal(t, "test", labels["env"])
+	assert.Equal(t, "e2e", labels["team"])

-	assert.Contains(t, fmt.Sprint(route["plugins"]), "proxy-rewrite")
+	plugins, ok := route["plugins"].(map[string]interface{})
+	require.True(t, ok, "plugins should be an object")
+	_, ok = plugins["proxy-rewrite"]
+	assert.True(t, ok)

Also applies to: 152-160, 192-196

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/e2e/route_test.go` around lines 113 - 117, The tests use fmt.Sprint(...)
to check JSON fields which can hide type issues; update the assertions after
runA7JSON(t, env, &route, "route", "get", ...) to assert the actual typed values
for route["paths"], route["methods"], route["labels"], and route["plugins"]
instead of stringifying them: cast or type-assert route[...] to the expected Go
type (e.g., []interface{} or map[string]interface{}), then use
assert.Contains/assert.Equal on the slice elements or map entries (for example
convert route["paths"] to []interface{} and assert it contains "/test-updated"),
and do the same for methods, labels and plugins to ensure correct types and
contents; keep using runA7JSON and the same route variable names to locate the
checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@test/e2e/credential_test.go`:
- Around line 90-93: The test prints raw CLI stdout/stderr on failure which can
leak secrets; in the credential update test where runA7WithEnv is called with
args ("credential", "update", credID, "--consumer", username, "-f", updateFile,
"-g", gatewayGroup") replace the require.NoError assertion that currently
includes "stdout=%s stderr=%s" with a non-sensitive failure message (e.g.
"credential update failed") or a redacted/structured log, so remove stdout and
stderr from the require.NoError call in this test function and only assert
failure with a generic message using require.NoError(t, err, "credential update
failed") or equivalent.

In `@test/e2e/secret_test.go`:
- Around line 92-93: The test currently exposes raw stdout/stderr in the failure
message when calling runA7WithEnv(..., "secret", "update", ...), which can leak
tokens; change the assertion around the call to avoid printing stdout/stderr on
failure — use require.NoError(t, err) without the formatted stdout/stderr
message, or replace the diagnostic with a redacted summary that removes any
bearer/JWT/access_token values; target the call site using runA7WithEnv and the
require.NoError invocation (variables stdout, stderr, err) and ensure any
failure logs do not include the raw stdout/stderr.

In `@test/e2e/setup_test.go`:
- Around line 187-188: The assertions currently print raw CLI output
(stdout/stderr) on failure — replace those detailed messages with context-only
or redacted messages to avoid leaking secrets: change the require.NoError(t,
err, "stdout=%s stderr=%s", stdout, stderr) to simply require.NoError(t, err,
"command failed") (or use a small redaction helper to show only a
truncated/hashed summary), and change require.NoError(t,
json.Unmarshal([]byte(stdout), out), "stdout=%s", stdout) to require.NoError(t,
json.Unmarshal([]byte(stdout), out), "failed to parse command output") (or use
the same redaction helper for stdout); keep references to stdout, stderr,
json.Unmarshal, require.NoError, and out so you update the exact assertion
lines.

---

Nitpick comments:
In `@test/e2e/config_sync_test.go`:
- Around line 185-186: The test currently uses assert.Contains on the raw YAML
dump (variables dumped, routeID, svcID) which can yield false positives; instead
parse the dumped YAML (e.g., yaml.Unmarshal into a map[string]interface{} or a
small struct) and then assert the presence of svcID under the "services" keys
and routeID under the "routes" entries. Update the assertions in
config_sync_test.go to unmarshal dumped into a parsed structure and use direct
existence checks (lookup map keys or iterate parsed slices) for svcID and
routeID rather than substring matching.

In `@test/e2e/credential_test.go`:
- Around line 72-77: The test currently checks plugins by doing
fmt.Sprint(credential["plugins"]) and substring matching which is brittle;
instead, after calling runA7JSON and populating the credential map, assert the
plugin structure directly by casting credential["plugins"] to the expected
collection type (e.g., []interface{} or map[string]interface{}), iterate its
entries and assert that one entry has the plugin identifier (e.g.,
plugin["name"] == "key-auth" or plugin["id"] matching expected) using
assert.True/assert.Equal; update both occurrences around the
runA7JSON/credential usage so you validate plugin-map membership structurally
rather than via string matching.

In `@test/e2e/global_rule_test.go`:
- Around line 66-70: The test currently checks plugins via
fmt.Sprint(globalRule["plugins"]) which inspects rendered text; instead,
type-assert globalRule["plugins"] to its decoded form (e.g.,
map[string]interface{} or []interface{}) and assert the presence of the
"prometheus" plugin key/value directly (for a map: assert.Contains(t,
pluginsMap, "prometheus"); for a slice: iterate and check item names). Update
the check in the globalRule assertion block (variable globalRule and call
runA7JSON) and apply the same change to the other occurrences mentioned (the
checks around lines 85-86 and 126-130) so assertions operate on the decoded
structure rather than fmt.Sprint output.

In `@test/e2e/plugin_metadata_test.go`:
- Around line 57-60: Replace the brittle stringified map checks that use
fmt.Sprint(metadata) with direct structural assertions against the parsed map
returned by runA7JSON; locate the test variables (metadata) and the call
runA7JSON(t, env, &metadata, "plugin-metadata", "get", pluginName, "-g",
gatewayGroup, "-o", "json") and assert the presence of keys and nested fields by
performing safe type assertions (e.g. check metadata is a
map[string]interface{}, then check nested map keys like "log_format" and
"client_ip" exist and are non-nil/expected) using
assert.NotNil/assert.True/assert.Contains or assert.Equal as appropriate rather
than relying on fmt.Sprint.

In `@test/e2e/route_test.go`:
- Around line 113-117: The tests use fmt.Sprint(...) to check JSON fields which
can hide type issues; update the assertions after runA7JSON(t, env, &route,
"route", "get", ...) to assert the actual typed values for route["paths"],
route["methods"], route["labels"], and route["plugins"] instead of stringifying
them: cast or type-assert route[...] to the expected Go type (e.g.,
[]interface{} or map[string]interface{}), then use assert.Contains/assert.Equal
on the slice elements or map entries (for example convert route["paths"] to
[]interface{} and assert it contains "/test-updated"), and do the same for
methods, labels and plugins to ensure correct types and contents; keep using
runA7JSON and the same route variable names to locate the checks.

In `@test/e2e/service_test.go`:
- Line 160: The test currently asserts on a stringified plugins value
(fmt.Sprint(service["plugins"])) which is brittle; instead decode
service["plugins"] into its concrete structure (e.g. map[string]interface{} or
[]interface{}) and assert the presence of the "proxy-rewrite" key/entry
directly. Locate the assertion using service["plugins"] in the test (around the
assert.Contains call) and replace it with a structural check that verifies the
key exists (e.g., type-assert to map and check map["proxy-rewrite"] or iterate a
slice and match the plugin name) so the test is robust against formatting
changes.

In `@test/e2e/ssl_test.go`:
- Line 83: The test currently converts ssl["snis"] to a string via fmt.Sprint
and asserts substring membership; instead decode the value as a typed slice and
assert membership on that slice. Locate the test variable ssl and replace the
two assertions that use fmt.Sprint(ssl["snis"]) with code that type-asserts
ssl["snis"] to []interface{} (or []string), map/convert it to []string (e.g.,
iterate and cast each element), and then use assert.Contains(t, convertedSlice,
"e2e-test.example.com") (or assert.True with a simple loop membership check) for
both occurrences so the test checks the actual array contents rather than
stringified output.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 191a08d6-ebff-4216-8cc6-c4dc0c5b7450

📥 Commits

Reviewing files that changed from the base of the PR and between ce83883 and 2982b63.

📒 Files selected for processing (13)
  • test/e2e/config_sync_test.go
  • test/e2e/consumer_test.go
  • test/e2e/credential_test.go
  • test/e2e/global_rule_test.go
  • test/e2e/plugin_metadata_test.go
  • test/e2e/proto_test.go
  • test/e2e/route_test.go
  • test/e2e/secret_test.go
  • test/e2e/service_template_test.go
  • test/e2e/service_test.go
  • test/e2e/setup_test.go
  • test/e2e/ssl_test.go
  • test/e2e/stream_route_test.go

Comment thread test/e2e/credential_test.go Outdated
Comment thread test/e2e/secret_test.go Outdated
Comment thread test/e2e/setup_test.go Outdated
Copilot AI review requested due to automatic review settings April 28, 2026 09:28
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Strengthens the E2E suite by switching several CRUD tests from substring-based checks to structured get -o json readback assertions, and by verifying deletion cleanup via post-delete get failures. This improves confidence that the CLI both writes and reads back the expected resource state in a real API7 EE environment.

Changes:

  • Introduce shared runA7JSON + JSON type assertion helpers for E2E tests.
  • Update multiple E2E CRUD tests to assert on decoded JSON fields after create/update, and to verify deletion via readback failures.
  • Extend config sync coverage by dumping remote config post-sync and asserting the created service/route are present.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
test/e2e/setup_test.go Adds shared runA7JSON, requireJSONObject, and requireJSONArray helpers for JSON-based assertions.
test/e2e/service_test.go Replaces substring checks with JSON field assertions; adds post-delete get failure check.
test/e2e/route_test.go Strengthens route assertions using decoded JSON (including arrays/maps); adds post-delete get failure check.
test/e2e/consumer_test.go Uses JSON assertions for get/update verification; adds post-delete get failure check.
test/e2e/credential_test.go Uses JSON assertions for plugin presence; adds post-delete get failure check.
test/e2e/ssl_test.go Uses JSON assertions for snis; adds post-delete get failure check.
test/e2e/secret_test.go Uses JSON readback assertions; adds update readback verification and post-delete get failure check.
test/e2e/global_rule_test.go Uses JSON assertions for plugin config persistence; adds post-delete get failure check.
test/e2e/plugin_metadata_test.go Uses JSON assertions (handling nested metadata shapes); adds post-delete get failure check.
test/e2e/proto_test.go Uses JSON assertions for content; adds post-delete get failure check.
test/e2e/stream_route_test.go Uses JSON assertions for stream route fields; adds update readback verification and post-delete get failure check.
test/e2e/service_template_test.go Uses JSON assertions for template fields; adds post-delete get failure check.
test/e2e/config_sync_test.go After sync, verifies route via JSON and validates presence via config dump YAML parsing.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread test/e2e/setup_test.go
Comment on lines +183 to +194
// runA7JSON executes a7 and decodes stdout as JSON. Callers should pass
// command arguments that already request JSON output, usually with "-o json".
func runA7JSON(t testTB, env []string, out interface{}, args ...string) string {
t.Helper()
stdout, _, err := runA7WithEnv(env, args...)
require.NoError(t, err, "a7 command failed: args=%v", args)

value := reflect.ValueOf(out)
require.True(t, value.Kind() == reflect.Ptr && !value.IsNil(), "out must be a non-nil pointer")
value.Elem().Set(reflect.Zero(value.Elem().Type()))

require.NoError(t, json.Unmarshal([]byte(stdout), out), "a7 returned non-JSON output: args=%v", args)
@guoqqqi guoqqqi merged commit 6bd6cbd into master Apr 28, 2026
6 checks passed
@guoqqqi guoqqqi deleted the codex/e2e-control-plane-readback branch April 28, 2026 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants