test: add utility command ginkgo e2e#30
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 (1)
✨ 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
Adds new Ginkgo-based E2E coverage for “utility” CLI commands (completion/version/extension/context) to align more of the E2E suite with the Ginkgo runner.
Changes:
- Introduces Ginkgo specs for
completionacross supported shells. - Adds Ginkgo specs for
version(text + JSON output). - Adds Ginkgo specs for
extensionandcontextcommand flows using an isolatedA6_CONFIG_DIR.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var _ = Describe("completion command", func() { | ||
| It("generates shell completions for supported shells", func() { | ||
| g := NewWithT(GinkgoT()) | ||
|
|
There was a problem hiding this comment.
These Ginkgo E2E specs largely duplicate existing non-Ginkgo E2E tests (e.g., test/e2e/completion_version_test.go, test/e2e/extension_test.go, test/e2e/context_test.go). Since all of these run under go test -tags e2e ./test/e2e, this will execute the same assertions twice, increasing suite time and maintenance burden. Consider migrating/removing the older testing.T variants (or gating one set behind a separate build tag) so coverage isn't duplicated.
There was a problem hiding this comment.
Acknowledged. This duplication is intentional for this phase: per the current test migration boundary, we are keeping the existing tests for now and adding Ginkgo E2E coverage first. We will decide separately whether to remove or gate the older variants after the Ginkgo coverage is in place.
| stdout, stderr, err = runA6WithEnv(env, "extension", "upgrade", "--all") | ||
| combined := stdout + stderr | ||
| g.Expect(err == nil || strings.Contains(combined, "No extensions") || strings.Contains(combined, "no extensions")).To(BeTrue(), | ||
| "stdout=%s stderr=%s err=%v", stdout, stderr, err) |
There was a problem hiding this comment.
The extension upgrade --all assertion is effectively a no-op right now: it passes whenever err == nil and doesn't check for the expected empty-state message. Since upgrade --all with no installed extensions currently prints "All extensions are already up to date" and returns nil, this test should assert err is nil and validate the expected output (or explicitly assert one of the known acceptable messages).
There was a problem hiding this comment.
Fixed in 181bd75 by asserting the command succeeds and checking the known empty-state output instead of accepting any nil error.
Summary
Verification