test(e2e): run invoice edit flows with credits enabled - #4589
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughSplits e2e execution into credits-enabled and credits-disabled phases, adds billing-worker compose support, enables credits in default configs, moves compatibility tests into a new package, adds settlement-mode and ledger-account coverage, and removes the moved settlement test from the main e2e file. ChangesCredits E2E Split and Billing Worker Infrastructure
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
Contributor
Greptile SummaryThis PR makes credits the default e2e runtime while keeping credits-disabled compatibility coverage. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (7): Last reviewed commit: "test: use stdlib invoice search helper" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Member
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yaml:
- Around line 799-803: The worker readiness checks in the CI workflow use curl
in the wait steps without failing on non-2xx HTTP responses, so a 503 from
/healthz can still pass. Update the curl commands in the wait-for-ready steps to
include --fail alongside the existing retry options, and apply the same change
to both readiness blocks so the tests only proceed once the workers are actually
healthy.
In `@e2e/creditsdisabled/settlement_mode_test.go`:
- Around line 69-94: The negative settlement-mode checks in the credits-disabled
tests only assert HTTP 400, so they can pass for the wrong validation path.
Update the relevant assertions in the custom subscription test in
settlement_mode_test.go (and the other credits-disabled cases it matches) to
verify the response body/error detail contains "credits are not enabled on this
deployment of OpenMeter" using the existing CreateSubscriptionWithResponse flow,
so the guard in CreateSubscription is explicitly covered.
In `@e2e/creditsdisabled/setup_test.go`:
- Around line 46-57: The helper is assuming the 201 response body always decodes
into resp.JSON201 and its UsageAttribution field, which can panic if the payload
is empty or malformed. In the setup helper that calls
client.UpsertUsageAttributionWithResponse, add require.NotNil checks for
resp.JSON201 and resp.JSON201.UsageAttribution before accessing SubjectKeys, and
keep the existing status/assertion flow so failures stay readable.
In `@e2e/ledger_accounts_test.go`:
- Around line 32-38: The current assertion in the ledger_accounts_test.go flow
only verifies that business accounts exist after customer creation, but it does
not prove they were provisioned by that create path. Update the test around
getBusinessAccountIDs and the customer-create call to either use an isolated
namespace or capture a before/after snapshot of ledger.BusinessAccountTypes so
the test asserts a real delta for the new customer provisioning path. Keep the
existing account-type checks, but make them validate the accounts were created
by this specific test scenario rather than preexisting state.
In `@e2e/Makefile`:
- Around line 59-63: The wait-local target uses curl health checks that can
succeed on 4xx/5xx, so the suite may continue before services are actually
ready. Update both readiness probe commands in the wait-local rule to use curl’s
failure-on-HTTP-error behavior so non-2xx responses keep retrying. Keep the
change localized to the wait-local target and preserve the existing
retry/timeouts.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d5b145ea-50f5-450d-9831-ffbb30716b07
📒 Files selected for processing (13)
.github/workflows/ci.yamle2e/Makefilee2e/config.yamle2e/creditsdisabled/README.mde2e/creditsdisabled/ledger_backfill_test.goe2e/creditsdisabled/settlement_mode_test.goe2e/creditsdisabled/setup_test.goe2e/docker-compose.credits-disabled.yamle2e/docker-compose.openmeter-local.yamle2e/docker-compose.openmeter.yamle2e/ledger_accounts_test.goe2e/productcatalog_test.goquickstart/config.yaml
💤 Files with no reviewable changes (1)
- e2e/productcatalog_test.go
turip
force-pushed
the
feat/invoice-edit-etoe
branch
2 times, most recently
from
June 28, 2026 07:06
57d94db to
f4c9c51
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@e2e/Makefile`:
- Around line 31-47: The teardown in the test-local-credits-disabled and
test-local-base targets only runs after successful commands, so failures in
wait-local or the test targets leave the compose stack running. Update these
Makefile targets to use a single shell flow with an EXIT trap or equivalent
always-run cleanup so env-local-down is executed even when setup or tests fail.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: eb9dda33-0fa7-47f4-a419-3b6b24fb91de
📒 Files selected for processing (14)
.github/workflows/ci.yamle2e/Makefilee2e/billinginvoices_v3_test.goe2e/config.yamle2e/creditsdisabled/README.mde2e/creditsdisabled/ledger_backfill_test.goe2e/creditsdisabled/settlement_mode_test.goe2e/creditsdisabled/setup_test.goe2e/docker-compose.credits-disabled.yamle2e/docker-compose.openmeter-local.yamle2e/docker-compose.openmeter.yamle2e/ledger_accounts_test.goe2e/productcatalog_test.goquickstart/config.yaml
💤 Files with no reviewable changes (1)
- e2e/productcatalog_test.go
✅ Files skipped from review due to trivial changes (4)
- e2e/creditsdisabled/README.md
- e2e/billinginvoices_v3_test.go
- e2e/docker-compose.credits-disabled.yaml
- e2e/config.yaml
🚧 Files skipped from review as they are similar to previous changes (8)
- quickstart/config.yaml
- e2e/docker-compose.openmeter-local.yaml
- e2e/creditsdisabled/ledger_backfill_test.go
- e2e/creditsdisabled/setup_test.go
- .github/workflows/ci.yaml
- e2e/creditsdisabled/settlement_mode_test.go
- e2e/docker-compose.openmeter.yaml
- e2e/ledger_accounts_test.go
turip
force-pushed
the
feat/invoice-edit-etoe
branch
from
June 28, 2026 07:12
f4c9c51 to
46eaab4
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
e2e/billinginvoices_v3_test.go (1)
201-203: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer
slices.IndexFunchere (and the same pattern below).
This is just a simple scan, so the stdlib helper fits better thanlo.Find;locan stay focused on the pointer helpers in this test.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/billinginvoices_v3_test.go` around lines 201 - 203, Replace the `lo.Find` scan in the invoice test with the standard library helper `slices.IndexFunc`, and apply the same change to the matching search pattern elsewhere in this test. Update the surrounding logic in `billinginvoices_v3_test.go` so it uses the returned index to access the item instead of relying on `foundStandard`, keeping `lo` only for the pointer helpers used in this test.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@e2e/billinginvoices_v3_test.go`:
- Around line 201-203: Replace the `lo.Find` scan in the invoice test with the
standard library helper `slices.IndexFunc`, and apply the same change to the
matching search pattern elsewhere in this test. Update the surrounding logic in
`billinginvoices_v3_test.go` so it uses the returned index to access the item
instead of relying on `foundStandard`, keeping `lo` only for the pointer helpers
used in this test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e0cc9a90-a030-4e34-9e82-4589da7d9015
📒 Files selected for processing (1)
e2e/billinginvoices_v3_test.go
turip
force-pushed
the
feat/invoice-edit-etoe
branch
from
June 28, 2026 11:47
7148e87 to
2a9dc06
Compare
turip
enabled auto-merge (squash)
June 28, 2026 11:49
tothandras
approved these changes
Jun 28, 2026
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
2 participants
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
This is a chore PR that enables credits by default for the e2e runtime and keeps quickstart aligned with that default.
The legacy credits-disabled checks stay covered, but they now live in an explicit compatibility package that runs after the default e2e suite.
Changes
e2e/creditsdisabled.Why
Credits are becoming the default billing runtime. The e2e stack should exercise that path by default, while still preserving temporary pre-credit compatibility coverage until the credits-disabled mode can be removed.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation