test: migrate config and group commands to ginkgo e2e#25
Conversation
|
Warning Rate limit exceeded
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 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 configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Pull request overview
This PR migrates CLI command coverage for plugin-config, plugin-metadata, global-rule, and consumer-group from mock Admin API unit tests to Ginkgo-based E2E tests against a real APISIX Admin API, while retaining only local/unit validation tests that don’t require external services.
Changes:
- Added new Ginkgo E2E specs for plugin-config, plugin-metadata, global-rule, and consumer-group command flows (create/get/list/export/update/delete, plus common validation/not-found cases).
- Removed mock Admin API command tests for those resources and replaced some with “local” tests focused on CLI validation behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| test/e2e/plugin_metadata_ginkgo_test.go | New Ginkgo E2E coverage for plugin-metadata command. |
| test/e2e/plugin_config_ginkgo_test.go | New Ginkgo E2E coverage for plugin-config command. |
| test/e2e/global_rule_ginkgo_test.go | New Ginkgo E2E coverage for global-rule command. |
| test/e2e/consumer_group_ginkgo_test.go | New Ginkgo E2E coverage for consumer-group command. |
| pkg/cmd/pluginmetadata/create/create_test.go | Removes mock-http tests for plugin-metadata create. |
| pkg/cmd/pluginmetadata/create/create_local_test.go | Adds local test for required --file validation. |
| pkg/cmd/pluginmetadata/get/get_test.go | Removes mock-http tests for plugin-metadata get. |
| pkg/cmd/pluginmetadata/update/update_test.go | Removes mock-http tests for plugin-metadata update. |
| pkg/cmd/pluginmetadata/update/update_local_test.go | Adds local test for required --file validation. |
| pkg/cmd/pluginmetadata/delete/delete_test.go | Removes mock-http tests for plugin-metadata delete. |
| pkg/cmd/pluginconfig/create/create_test.go | Removes mock-http tests for plugin-config create. |
| pkg/cmd/pluginconfig/create/create_local_test.go | Keeps local/unit coverage for missing-id validation. |
| pkg/cmd/pluginconfig/get/get_test.go | Removes mock-http tests for plugin-config get. |
| pkg/cmd/pluginconfig/get/get_local_test.go | Keeps local/unit coverage for non-TTY arg validation. |
| pkg/cmd/pluginconfig/list/list_test.go | Removes mock-http tests for plugin-config list. |
| pkg/cmd/pluginconfig/update/update_test.go | Removes mock-http tests for plugin-config update. |
| pkg/cmd/pluginconfig/update/update_local_test.go | Keeps local/unit coverage for non-TTY arg validation. |
| pkg/cmd/pluginconfig/export/export_test.go | Removes mock-http tests for plugin-config export. |
| pkg/cmd/pluginconfig/delete/delete_test.go | Removes mock-http tests for plugin-config delete. |
| pkg/cmd/pluginconfig/delete/delete_local_test.go | Keeps local/unit coverage for flag interaction validation. |
| pkg/cmd/globalrule/create/create_test.go | Removes mock-http tests for global-rule create. |
| pkg/cmd/globalrule/create/create_local_test.go | Keeps local/unit coverage for missing-id validation. |
| pkg/cmd/globalrule/get/get_test.go | Removes mock-http tests for global-rule get. |
| pkg/cmd/globalrule/get/get_local_test.go | Keeps local/unit coverage for non-TTY arg validation. |
| pkg/cmd/globalrule/list/list_test.go | Removes mock-http tests for global-rule list. |
| pkg/cmd/globalrule/update/update_test.go | Removes mock-http tests for global-rule update. |
| pkg/cmd/globalrule/update/update_local_test.go | Keeps local/unit coverage for non-TTY arg validation. |
| pkg/cmd/globalrule/export/export_test.go | Removes mock-http tests for global-rule export. |
| pkg/cmd/globalrule/delete/delete_test.go | Removes mock-http tests for global-rule delete. |
| pkg/cmd/globalrule/delete/delete_local_test.go | Keeps local/unit coverage for flag interaction validation. |
| pkg/cmd/consumergroup/create/create_test.go | Removes mock-http tests for consumer-group create. |
| pkg/cmd/consumergroup/create/create_local_test.go | Keeps local/unit coverage for missing-id validation. |
| pkg/cmd/consumergroup/get/get_test.go | Removes mock-http tests for consumer-group get. |
| pkg/cmd/consumergroup/get/get_local_test.go | Keeps local/unit coverage for non-TTY arg validation. |
| pkg/cmd/consumergroup/list/list_test.go | Removes mock-http tests for consumer-group list. |
| pkg/cmd/consumergroup/update/update_test.go | Removes mock-http tests for consumer-group update. |
| pkg/cmd/consumergroup/update/update_local_test.go | Keeps local/unit coverage for non-TTY arg validation. |
| pkg/cmd/consumergroup/export/export_test.go | Removes mock-http tests for consumer-group export. |
| pkg/cmd/consumergroup/delete/delete_test.go | Removes mock-http tests for consumer-group delete. |
| pkg/cmd/consumergroup/delete/delete_local_test.go | Keeps local/unit coverage for flag interaction validation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var _ = Describe("plugin-metadata command", Ordered, func() { | ||
| var env []string | ||
|
|
||
| BeforeEach(func() { | ||
| env = setupPluginMetadataEnvWithKey(NewWithT(GinkgoT()), adminKey) | ||
| }) | ||
|
|
||
| Describe("create", func() { |
There was a problem hiding this comment.
These new Ginkgo specs appear to overlap significantly with the existing testify-based E2E tests for the same command (e.g., test/e2e/plugin_metadata_test.go). If the intent is to migrate coverage to the Ginkgo suite as described in the PR summary, consider removing or disabling the legacy E2E tests to avoid duplicated runtime and potentially conflicting resource IDs.
| func setupGlobalRuleEnvWithKey(g Gomega, apiKey string) []string { | ||
| env := []string{ | ||
| "A6_CONFIG_DIR=" + GinkgoT().TempDir(), | ||
| } | ||
| _, stderr, err := runA6WithEnv(env, "context", "create", "test", | ||
| "--server", adminURL, "--api-key", apiKey) | ||
| g.Expect(err).NotTo(HaveOccurred(), "failed to create global-rule test context: %s", stderr) | ||
| return env | ||
| } | ||
|
|
||
| func writeGlobalRuleFile(g Gomega, name, body string) string { | ||
| path := filepath.Join(GinkgoT().TempDir(), name) | ||
| g.Expect(os.WriteFile(path, []byte(body), 0o644)).To(Succeed()) | ||
| return path | ||
| } | ||
|
|
||
| func deleteGlobalRuleViaAdminByID(g Gomega, id string) { | ||
| resp, err := adminAPI("DELETE", "/apisix/admin/global_rules/"+id, nil) | ||
| if err == nil && resp != nil { | ||
| g.Expect(resp.Body.Close()).To(Succeed()) | ||
| } | ||
| } | ||
|
|
||
| func deleteGlobalRuleViaCLIByID(env []string, id string) { | ||
| _, _, _ = runA6WithEnv(env, "global-rule", "delete", id, "--force") | ||
| } | ||
|
|
||
| func createGlobalRuleViaCLIFile(g Gomega, env []string, file string) { | ||
| stdout, stderr, err := runA6WithEnv(env, "global-rule", "create", "-f", file) | ||
| g.Expect(err).NotTo(HaveOccurred(), "stdout=%s stderr=%s", stdout, stderr) | ||
| } |
There was a problem hiding this comment.
This file duplicates helper patterns that already exist in test/e2e/ginkgo_helpers_test.go (setupCLIEnvWithKey, writeTempFile/write*File, and deleteResourceViaAdminByID). Reusing the shared helpers would reduce maintenance overhead and keep the E2E suites consistent.
| func setupConsumerGroupEnvWithKey(g Gomega, apiKey string) []string { | ||
| env := []string{ | ||
| "A6_CONFIG_DIR=" + GinkgoT().TempDir(), | ||
| } | ||
| _, stderr, err := runA6WithEnv(env, "context", "create", "test", | ||
| "--server", adminURL, "--api-key", apiKey) | ||
| g.Expect(err).NotTo(HaveOccurred(), "failed to create consumer-group test context: %s", stderr) | ||
| return env | ||
| } | ||
|
|
||
| func writeConsumerGroupFile(g Gomega, name, body string) string { | ||
| path := filepath.Join(GinkgoT().TempDir(), name) | ||
| g.Expect(os.WriteFile(path, []byte(body), 0o644)).To(Succeed()) | ||
| return path | ||
| } | ||
|
|
||
| func deleteConsumerGroupViaAdminByID(g Gomega, id string) { | ||
| resp, err := adminAPI("DELETE", "/apisix/admin/consumer_groups/"+id, nil) | ||
| if err == nil && resp != nil { | ||
| g.Expect(resp.Body.Close()).To(Succeed()) | ||
| } | ||
| } | ||
|
|
||
| func deleteConsumerGroupViaCLIByID(env []string, id string) { | ||
| _, _, _ = runA6WithEnv(env, "consumer-group", "delete", id, "--force") | ||
| } | ||
|
|
||
| func createConsumerGroupViaCLIFile(g Gomega, env []string, file string) { | ||
| stdout, stderr, err := runA6WithEnv(env, "consumer-group", "create", "-f", file) | ||
| g.Expect(err).NotTo(HaveOccurred(), "stdout=%s stderr=%s", stdout, stderr) | ||
| } |
There was a problem hiding this comment.
This file duplicates helper patterns that already exist in test/e2e/ginkgo_helpers_test.go (setupCLIEnvWithKey, writeTempFile/write*File, and deleteResourceViaAdminByID). Consider using the shared helpers (with resource path "/apisix/admin/consumer_groups") to reduce duplication and keep E2E setup/cleanup consistent.
| var _ = Describe("plugin-config command", Ordered, func() { | ||
| var env []string | ||
|
|
||
| BeforeEach(func() { | ||
| env = setupPluginConfigEnvWithKey(NewWithT(GinkgoT()), adminKey) | ||
| }) | ||
|
|
There was a problem hiding this comment.
These new Ginkgo specs appear to duplicate existing testify-based E2E coverage for the same command (e.g., test/e2e/plugin_config_test.go). If the PR goal is a migration to Ginkgo E2E, consider removing the legacy tests to avoid doubling suite runtime and reducing the chance of resource ID collisions.
| func setupPluginMetadataEnvWithKey(g Gomega, apiKey string) []string { | ||
| env := []string{ | ||
| "A6_CONFIG_DIR=" + GinkgoT().TempDir(), | ||
| } | ||
| _, stderr, err := runA6WithEnv(env, "context", "create", "test", | ||
| "--server", adminURL, "--api-key", apiKey) | ||
| g.Expect(err).NotTo(HaveOccurred(), "failed to create plugin-metadata test context: %s", stderr) | ||
| return env | ||
| } | ||
|
|
||
| func writePluginMetadataFile(g Gomega, name, body string) string { | ||
| path := filepath.Join(GinkgoT().TempDir(), name) | ||
| g.Expect(os.WriteFile(path, []byte(body), 0o644)).To(Succeed()) | ||
| return path | ||
| } | ||
|
|
||
| func deletePluginMetadataViaAdminByName(g Gomega, pluginName string) { | ||
| resp, err := adminAPI("DELETE", "/apisix/admin/plugin_metadata/"+pluginName, nil) | ||
| if err == nil && resp != nil { | ||
| g.Expect(resp.Body.Close()).To(Succeed()) | ||
| } | ||
| } |
There was a problem hiding this comment.
This file reimplements helpers that already exist in test/e2e/ginkgo_helpers_test.go (setupCLIEnvWithKey, writeTempFile, and deleteResourceViaAdminByID). Consider reusing the shared helpers (and, for delete, deleteResourceViaAdminByID with the plugin_metadata resource path) to reduce duplication and keep E2E setup/cleanup consistent across command suites.
Summary
Verification