test(integration): config-aware billing equivalences + plan id (closes #3609)#3610
Conversation
…ions (#3609) - auth.integration: assert billing.equivalences against config.billing.equivalences (was hardcoded to null — broke any downstream that defines equivalences for /pricing) - billing.lifecycle.integration: derive plan ids from config.billing.plans for the starter→pro transition test (was hardcoded 'starter' — broke any downstream whose plan enum excludes starter, e.g. trawl with free/growth/pro) Tests still cover the upstream defaults (no planDefinitions → fallback to legacy 'starter'/'pro'; no equivalences → null) but no longer assume those values. Refs #3609.
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ 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: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughTwo integration tests are updated to use dynamic values from billing config instead of hard-coded expectations. Auth test assertion for ChangesConfig-Aware Integration Tests
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Duplication | 0 |
AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.
TIP This summary will be updated as you push new changes.
There was a problem hiding this comment.
Pull Request Overview
This PR successfully transitions the integration test suite from hardcoded billing plans to configuration-driven values, ensuring compatibility across different environments. Codacy results indicate the changes are up to standards with no new issues reported. However, a logic flaw was identified in billing.lifecycle.integration.tests.js where the absence of billing configuration could cause a TypeError, contradicting the safer implementation found in auth.integration.tests.js.
About this PR
- There is a systemic inconsistency in how
config.billingis accessed. Whileauth.integration.tests.jsuses defensive checks,billing.lifecycle.integration.tests.jsaccesses the object directly. Standardizing on the defensive pattern used in the auth tests will prevent environment-specific test failures.
Test suggestions
- Verify auth integration response matches configured billing equivalences
- Verify billing lifecycle test correctly handles dynamic plan IDs from config
- Verify BillingUsage planVersion is updated correctly upon plan upgrade
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
… against undefined
There was a problem hiding this comment.
Pull request overview
Adjusts integration tests to be config-aware so downstream projects with customized billing configuration (e.g., custom billing.equivalences and non-starter plan enums) don’t fail during test:integration.
Changes:
- Update auth config integration assertion to match
config.billing.equivalences(defaulting tonull). - Update billing lifecycle integration test to select plan ids from
config.billing.plansrather than hardcoding'starter'.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| modules/billing/tests/billing.lifecycle.integration.tests.js | Derives initial/upgrade plan ids from config to avoid hardcoded upstream-only plan values. |
| modules/auth/tests/auth.integration.tests.js | Makes billing.equivalences assertion reflect the current config instead of assuming null. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3610 +/- ##
=======================================
Coverage 87.57% 87.57%
=======================================
Files 132 132
Lines 4049 4049
Branches 1260 1260
=======================================
Hits 3546 3546
Misses 390 390
Partials 113 113
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Summary
auth.integrationline 1545 — replacesexpect(...equivalences).toBeNull()withtoEqual(config.billing?.equivalences ?? null)so downstreams that define equivalences (e.g. trawl/pricing) no longer fail.billing.lifecycle.integrationfirst test — derives the initial/upgrade plan ids fromconfig.billing.plans(with legacy'starter'/'pro'fallback for upstream defaults) instead of hardcoding'starter'. The Subscription/Organization schema enum is locked at import time, so trawl's['free','growth','pro']enum was rejecting the literal'starter'.Why
The integration job split (commit
efbd7bcb) made these two failures visible to downstreams. trawl_node currently carries aDOWNSTREAM_PATCHES.mdoverride (PR comes-io/trawl_node#1128), which it can drop once this lands.Refs
feedback_update_stack_theirs_wipes_patches.md— second instance of the same pattern, durable fix is upstream.Test plan
npm run test:integration— 24 suites / 335 tests pass on upstream defaults (noplanDefinitions, noequivalences)/update-stackpropagation to trawl_node revert the DOWNSTREAM_PATCHES.md overrideCloses #3609
Summary by CodeRabbit
Release Notes