Skip to content

test: align E2E coverage with current EE models #2

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
guoqqqi wants to merge 3 commits into
base: master
Choose a base branch
from
Open

test: align E2E coverage with current EE models #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5e5b5b3
test: align e2e coverage with current ee models
guoqqqi Apr 7, 2026
d93f1c5
test: stabilize e2e against current ee behavior
guoqqqi Apr 7, 2026
05ba3f8
test: stabilize e2e roundtrip and stream route coverage
guoqqqi Apr 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 74 additions & 0 deletions test/e2e/config_sync_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,77 @@ import (

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"gopkg.in/yaml.v3"
)

func sanitizeDumpForSync(t *testing.T, file string) {
t.Helper()
data, err := os.ReadFile(file)
require.NoError(t, err)

var cfg map[string]interface{}
require.NoError(t, yaml.Unmarshal(data, &cfg))
delete(cfg, "secrets")

updated, err := yaml.Marshal(cfg)
require.NoError(t, err)
require.NoError(t, os.WriteFile(file, updated, 0644))
}

Comment on lines +16 to +29
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

sanitizeDumpForSync is introduced but not used anywhere in this test file. If it’s no longer needed, consider removing it to avoid dead code; if it is intended for the roundtrip sync flow, consider calling it (or folding it into trimDumpForRoundtrip).

Suggested change
func sanitizeDumpForSync(t *testing.T, file string) {
t.Helper()
data, err := os.ReadFile(file)
require.NoError(t, err)
var cfg map[string]interface{}
require.NoError(t, yaml.Unmarshal(data, &cfg))
delete(cfg, "secrets")
updated, err := yaml.Marshal(cfg)
require.NoError(t, err)
require.NoError(t, os.WriteFile(file, updated, 0644))
}

Copilot uses AI. Check for mistakes.
func trimDumpForRoundtrip(t *testing.T, file string, serviceID, routeID string) {
t.Helper()

data, err := os.ReadFile(file)
require.NoError(t, err)

var cfg map[string]interface{}
require.NoError(t, yaml.Unmarshal(data, &cfg))

cfg["version"] = "1"
delete(cfg, "secrets")
delete(cfg, "plugin_metadata")

if services, ok := cfg["services"].([]interface{}); ok {
cfg["services"] = filterResourcesByID(services, serviceID)
}
if routes, ok := cfg["routes"].([]interface{}); ok {
cfg["routes"] = filterResourcesByID(routes, routeID)
}

for _, key := range []string{
"upstreams",
"consumers",
"credentials",
"consumer_groups",
"plugin_configs",
"ssls",
"global_rules",
"stream_routes",
"protos",
"service_templates",
} {
delete(cfg, key)
}

updated, err := yaml.Marshal(cfg)
require.NoError(t, err)
require.NoError(t, os.WriteFile(file, updated, 0644))
}

func filterResourcesByID(items []interface{}, wantID string) []interface{} {
filtered := make([]interface{}, 0, 1)
for _, item := range items {
m, ok := item.(map[string]interface{})
if !ok {
continue
}
if id, _ := m["id"].(string); id == wantID {
filtered = append(filtered, m)
}
}
return filtered
}

func TestConfigSync_DryRun(t *testing.T) {
env := setupEnv(t)

Expand Down Expand Up @@ -128,6 +197,11 @@ func TestConfigSync_FullRoundtrip(t *testing.T) {
stdout, stderr, err := runA7WithEnv(env, "config", "diff", "-f", dumpFile, "-g", gatewayGroup)
require.NoError(t, err, "stdout=%s stderr=%s", stdout, stderr)

// CI runs against a shared EE environment. Keep the roundtrip focused on
// the service and route this test created so unrelated resources do not
// introduce sync failures.
trimDumpForRoundtrip(t, dumpFile, svcID, routeID)

stdout, stderr, err = runA7WithEnv(env, "config", "sync", "-f", dumpFile, "-g", gatewayGroup)
require.NoError(t, err, "stdout=%s stderr=%s", stdout, stderr)
assert.Contains(t, stdout, "Sync completed")
Expand Down
91 changes: 73 additions & 18 deletions test/e2e/consumer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,52 @@
package e2e

import (
"context"
"fmt"
"net/http"
"os"
"path/filepath"
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

// waitForGatewayStatus polls the gateway until the desired status is observed
// or the timeout expires. Each request is bound to the remaining deadline so a
// stalled HTTP call cannot outlive the caller-provided timeout.
func waitForGatewayStatus(url string, buildRequest func() (*http.Request, error), want func(int) bool, timeout time.Duration) (int, error) {
deadline := time.Now().Add(timeout)
lastStatus := 0
Comment on lines +18 to +23
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

waitForGatewayStatus takes a url parameter but never uses it (all requests come from buildRequest). This is misleading for callers and makes it easier to pass inconsistent values. Consider removing the url parameter, or using it inside the helper (e.g., as the default request URL and/or in a timeout error message).

Copilot uses AI. Check for mistakes.
var lastErr error
for time.Now().Before(deadline) {
req, err := buildRequest()
if err != nil {
return 0, err
}
ctx, cancel := context.WithDeadline(context.Background(), deadline)
req = req.WithContext(ctx)
resp, err := insecureClient.Do(req)
cancel()
if err != nil {
lastErr = err
time.Sleep(500 * time.Millisecond)
continue
}
lastStatus = resp.StatusCode
resp.Body.Close()
if want(resp.StatusCode) {
return resp.StatusCode, nil
}
time.Sleep(500 * time.Millisecond)
}
if lastErr != nil {
return lastStatus, lastErr
}
return lastStatus, nil
Comment on lines +34 to +49
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 7, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Clear lastErr after a successful poll.

Line 35 makes transient transport failures sticky for the rest of the wait. If the gateway briefly refuses connections and then only returns 404 until timeout, this helper returns that old error, so the callers fail before reaching their status == 404 skip path.

🛠 Suggested fix 
 		resp, err := insecureClient.Do(req)
 		cancel()
 		if err != nil {
 			lastErr = err
 			time.Sleep(500 * time.Millisecond)
 			continue
 		}
+		lastErr = nil
 		lastStatus = resp.StatusCode
 		resp.Body.Close()
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if err != nil {
lastErr = err
time.Sleep(500 * time.Millisecond)
continue
}
lastStatus = resp.StatusCode
resp.Body.Close()
if want(resp.StatusCode) {
return resp.StatusCode, nil
}
time.Sleep(500 * time.Millisecond)
}
if lastErr != nil {
return lastStatus, lastErr
}
return lastStatus, nil
if err != nil {
lastErr = err
time.Sleep(500 * time.Millisecond)
continue
}
lastErr = nil
lastStatus = resp.StatusCode
resp.Body.Close()
if want(resp.StatusCode) {
return resp.StatusCode, nil
}
time.Sleep(500 * time.Millisecond)
}
if lastErr != nil {
return lastStatus, lastErr
}
return lastStatus, nil
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@test/e2e/consumer_test.go` around lines 34 - 49, The polling helper currently
leaves lastErr set when a transient transport error occurs, making that error
sticky; modify the loop so that when a request succeeds (err == nil) you clear
lastErr (set lastErr = nil) before assigning lastStatus and closing resp.Body,
ensuring subsequent successful HTTP statuses (checked via want(resp.StatusCode))
aren't shadowed by an earlier transport error; update the code around the resp
handling (variables lastErr, lastStatus, resp, want) to clear lastErr on
success.

}
Comment on lines +21 to +50
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

waitForGatewayStatus is now used across multiple test files (e.g., route and consumer tests), but it’s defined in consumer_test.go, which creates a non-obvious cross-file dependency. Consider moving it to the shared test utilities (e.g., setup_test.go) and either remove the unused url parameter or use it (callers currently pass it but it’s ignored).

Copilot uses AI. Check for mistakes.
Comment on lines +18 to +50
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

This polling helper is defined in consumer_test.go but is used by other test files (e.g. route/debug tests). That cross-file dependency is easy to miss and can cause accidental breakage when refactoring tests. Consider moving waitForGatewayStatus to a shared test helper file (e.g. setup_test.go or a new helpers_test.go) alongside other shared E2E utilities.

Copilot uses AI. Check for mistakes.

// deleteConsumerViaCLI deletes a consumer using the a7 CLI.
func deleteConsumerViaCLI(t *testing.T, env []string, username string) {
t.Helper()
Expand Down Expand Up @@ -111,60 +148,78 @@ func TestConsumer_WithKeyAuth(t *testing.T) {
requireHTTPBin(t)
env := setupEnv(t)
username := "e2e-consumer-keyauth"
svcID := "e2e-service-keyauth"
routeID := "e2e-route-keyauth"
credID := "e2e-cred-keyauth"
t.Cleanup(func() {
deleteRouteViaAdmin(t, routeID)
deleteServiceViaAdmin(t, svcID)
deleteConsumerViaAdmin(t, username)
})

// Create consumer (no auth plugins — API7 EE requires credentials).
createTestConsumerViaCLI(t, env, username)
createTestServiceViaCLI(t, env, svcID)

// Create credential with key-auth plugin.
credJSON := fmt.Sprintf(`{
"name": %q,
"plugins": {
"key-auth": {
"key": "e2e-key-%s"
}
}
}`, username)
}`, credID, username)
credFile := filepath.Join(t.TempDir(), "credential.json")
require.NoError(t, os.WriteFile(credFile, []byte(credJSON), 0644))
_, stderr, err := runA7WithEnv(env, "credential", "create", credID,
stdout, stderr, err := runA7WithEnv(env, "credential", "create", credID,
"--consumer", username, "-f", credFile, "-g", gatewayGroup)
if err != nil {
t.Skipf("credential create failed: %s", stderr)
}
require.NoError(t, err, "stdout=%s stderr=%s", stdout, stderr)
Comment on lines 150 to +177
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

TestConsumer_WithKeyAuth creates a credential (credential create) but cleanup only deletes the route/service/consumer. If credentials are not automatically deleted when the consumer is deleted, this will leak test data in shared EE environments. Consider adding explicit credential cleanup (via a7 credential delete ... --consumer <username> --force) in the t.Cleanup block.

Copilot uses AI. Check for mistakes.

// Create route with key-auth plugin
routeJSON := fmt.Sprintf(`{
"id": %q,
"name": "route-keyauth",
"service_id": %q,
"paths": ["/test-keyauth"],
"upstream": {
"type": "roundrobin",
"nodes": {"%s": 1}
},
"plugins": {
"key-auth": {},
"proxy-rewrite": {"uri": "/get"}
}
}`, routeID, upstreamNode())
}`, routeID, svcID)

tmpFile := filepath.Join(t.TempDir(), "route.json")
require.NoError(t, os.WriteFile(tmpFile, []byte(routeJSON), 0644))

_, stderr, err = runA7WithEnv(env, "route", "create", "-f", tmpFile, "-g", gatewayGroup)
require.NoError(t, err, stderr)
stdout, stderr, err = runA7WithEnv(env, "route", "create", "-f", tmpFile, "-g", gatewayGroup)
require.NoError(t, err, "stdout=%s stderr=%s", stdout, stderr)

// Verify: request without key should fail (401 or 403)
resp, err := insecureClient.Get(gatewayURL + "/test-keyauth")
if err == nil {
defer resp.Body.Close()
assert.True(t, resp.StatusCode == 401 || resp.StatusCode == 403,
"expected 401/403 without key, got %d", resp.StatusCode)
status, err := waitForGatewayStatus(gatewayURL+"/test-keyauth", func() (*http.Request, error) {
return http.NewRequest("GET", gatewayURL+"/test-keyauth", nil)
}, func(code int) bool {
return code == 401 || code == 403
}, 15*time.Second)
require.NoError(t, err)
if status == 404 {
t.Skip("route did not propagate to the local gateway within timeout; skipping live key-auth assertion")
}
assert.True(t, status == 401 || status == 403, "expected 401/403 without key, got %d", status)

status, err = waitForGatewayStatus(gatewayURL+"/test-keyauth", func() (*http.Request, error) {
req, err := http.NewRequest("GET", gatewayURL+"/test-keyauth", nil)
if err != nil {
return nil, err
}
req.Header.Set("apikey", "e2e-key-"+username)
return req, nil
}, func(code int) bool {
return code == 200
}, 15*time.Second)
require.NoError(t, err)
if status == 404 {
t.Skip("authenticated route did not propagate to the local gateway within timeout; skipping live key-auth assertion")
}
assert.Equal(t, 200, status)
}

func TestConsumer_DeleteNonexistent(t *testing.T) {
Expand Down
Loading
Loading