test(integration): skip tier-gated suites when GitLab license unavailable#429
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds Jest global setup tier detection and caching, a tierGate module exporting detection and test wrappers, and gates Premium/Ultimate-dependent integration tests to skip when the detected tier doesn't meet requirements. ChangesTier-conditional integration test execution
Sequence Diagram(s)sequenceDiagram
participant GlobalSetup
participant GitLabGraphQL
participant TempFile
participant TierGate
GlobalSetup->>GitLabGraphQL: POST { currentLicense { plan } } (10s timeout)
GitLabGraphQL-->>GlobalSetup: 200 + plan string
GlobalSetup->>TempFile: write { tier, plan } namespaced by REPO_HASH
TierGate->>TempFile: read cached JSON at module load
TempFile-->>TierGate: cached payload
TierGate->>TierGate: validate schema -> set detected tier (or 'unknown')
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 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 docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
This PR updates the integration-test harness to treat GitLab license/tier limitations as an environment constraint by skipping Premium/Ultimate-only suites (pending) instead of failing them when the target instance is Free/unlicensed. It adds a small tier-gating utility plus a one-time tier detection step in Jest global setup, then applies the gating to the known tier-dependent integration suites.
Changes:
- Add
describeIfTier/itIfTierhelpers that synchronously read a detected tier from a tmpfile at test parse time. - Extend integration
globalSetupto querycurrentLicense { plan }once and persist the detected tier for Jest workers. - Gate Ultimate/Premium-only integration suites (requirements verification, debug widget assignment, and Epic-dependent parts of data lifecycle).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
tests/setup/tierGate.ts |
New helper to conditionally describe.skip / it.skip based on detected tier. |
tests/setup/globalSetup.js |
Adds one-time GraphQL tier detection and writes result to a tmpfile for synchronous access by workers. |
tests/integration/requirements.test.ts |
Wraps suite with describeIfTier('ultimate', ...) to skip on non-Ultimate instances. |
tests/integration/debug-widget-assignment.test.ts |
Wraps suite with describeIfTier('premium', ...) to skip on Free. |
tests/integration/data-lifecycle.test.ts |
Gates Epic-dependent steps with itIfTier/describeIfTier('premium', ...) while leaving tier-agnostic steps runnable everywhere. |
…able Add tierGate helper that detects the target GitLab tier once in globalSetup (currentLicense GraphQL query) and writes it to a tmp file for synchronous read at test parse time. Provides describeIfTier / itIfTier wrappers that emit describe.skip / it.skip with a clear "[skipped: requires X, detected Y]" suffix when the detected tier is below the required minimum. Apply to the suites that currently hard-fail on Free instances: - requirements.test.ts: VERIFICATION_STATUS widget is Ultimate-only - debug-widget-assignment.test.ts: Epics + colour widget are Premium+ - data-lifecycle.test.ts: GROUP-level Epics + subgroup parent-epic + Epic- filtered list_work_items assertions are Premium+ (other steps still run) These suites now report as pending on Free, not failed — the underlying behaviour is "feature unavailable in this environment", not "code broken". Closes #428
…rror catch - Tier cache filename now includes a sha1(repo_root) prefix so concurrent Jest runs across worktrees (or NFS-shared tmpdirs) cannot clobber each other's detection result. - Tier detection catch formats the error defensively (`err instanceof Error ? err.message : String(err)`) so a non-Error throw cannot itself crash global setup. - tierGate.ts mirrors the same namespacing so workers read the right file.
polaz
force-pushed
the
test/#428-tier-gating-integration-suites
branch
from
5afe7bf to
79882ce
Compare
SonarCloud flags crypto.createHash('sha1') as a weak-hash security hotspot
regardless of context. The hash here is a cache-key digest of the checkout
path (used to namespace the tier-detection tmp file across worktrees), not
a security primitive. Switch to sha256 to silence the false positive while
keeping the namespacing semantics identical.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
tests/setup/globalSetup.js:76
- If the GraphQL request returns a non-2xx status (or returns
{ errors: [...] }), the code currently treats it asplan=''and logs a successful detection offree. That makes auth/URL/server failures indistinguishable from an actual Free tier and can hide real regressions by skipping gated tests. Consider treating non-OK / GraphQL errors as a detection failure (warn + fall back), and include status/errors in the warning for easier diagnosis.
});
const data = res.ok ? await res.json() : null;
const plan = (data?.data?.currentLicense?.plan ?? '').toLowerCase();
let tier = 'free';
if (plan.includes('ultimate') || plan.includes('gold')) tier = 'ultimate';
else if (plan.includes('premium') || plan.includes('silver')) tier = 'premium';
fs.writeFileSync(tierFile, JSON.stringify({ tier, plan }));
console.log(`🎫 Detected GitLab tier: ${tier}${plan ? ` (plan: ${plan})` : ''}`);
- Bearer header works for both PAT and OAuth tokens; PRIVATE-TOKEN would 401 on OAuth tokens and silently default to 'free', incorrectly skipping Premium/Ultimate suites when integration tests run in OAuth mode. - AbortController with 10s timeout prevents a hung TCP connection in globalSetup from blocking the entire suite startup indefinitely (Jest's per-test timeout doesn't apply at the globalSetup stage).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@tests/setup/globalSetup.js`:
- Around line 78-84: The setup currently treats non-2xx and GraphQL-error
responses as a “free” tier; change globalSetup.js so that if res.ok is false or
the parsed JSON contains data.errors or lacks data.currentLicense you fail the
setup instead of defaulting: after fetching, if (!res.ok) log the response
status/body and process.exit(1); otherwise parse JSON into data and if
(data.errors || !data?.data?.currentLicense) log the GraphQL error/missing field
and process.exit(1); only then compute plan and tier (using plan =
(data.data.currentLicense.plan ?? '').toLowerCase()) and write tierFile—this
ensures describeIfTier and tier-gated tests don't silently run on auth/API
failures.
In `@tests/setup/tierGate.ts`:
- Around line 47-53: The readDetectedTier function currently uses JSON.parse
plus manual checks when reading TIER_FILE; replace that ad-hoc parsing with a
Zod schema validation: import or define a TierCacheSchema (matching { tier?:
GitLabTier }) and use TierCacheSchema.safeParse on the file contents, then
return the validated tier if it is one of 'ultimate'|'premium'|'free', otherwise
return 'free'; ensure you handle missing file (fs.existsSync) the same way and
avoid throwing on invalid content by falling back to 'free'.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b9ed02ae-4ccd-4f34-a808-de455d09688a
📒 Files selected for processing (5)
tests/integration/data-lifecycle.test.tstests/integration/debug-widget-assignment.test.tstests/integration/requirements.test.tstests/setup/globalSetup.jstests/setup/tierGate.ts
…rden cache IO
- !res.ok and data.errors now throw into the catch path so auth/network
breakage logs a warning instead of silently defaulting to 'free' and
skipping every Premium/Ultimate suite
- fs.rmSync(..., { force: true }) replaces unlinkSync to survive Windows
file locks / EPERM without crashing the integration run
- tierGate.ts validates the cross-process cache JSON with a Zod schema
instead of ad-hoc parse + manual checks
fs.rmSync({ force: true }) only suppresses ENOENT — Windows file locks
or permission errors would still throw and crash the entire integration
run before any tests start. The subsequent writeFileSync overwrites the
file anyway, so a failed unlink is not load-bearing; wrap in try/catch
with a brief warning and proceed. Adds maxRetries=2 for transient locks.
…f silent 'free' fallback
Defaulting to 'free' when tier detection fails (network/auth/GraphQL error)
made every Premium/Ultimate suite skip silently, producing a misleading
green run with many pending tests. The intent of the catch path is to
report failure, not to claim a Free instance.
- globalSetup.js writes { tier: 'unknown', detectionFailed: true, reason }
on any error in the detection flow
- TierCacheSchema extended to accept 'unknown' + new optional fields
- tierGate's internal type is DetectedTier = GitLabTier | 'unknown'
(public GitLabTier unchanged — required minimum is still free|premium|ultimate)
- tierSatisfies('unknown') returns true so describeIfTier / itIfTier do
NOT skip; the suite runs and fails loudly with the real underlying
error if the gated feature is genuinely unavailable
- readDetectedTier also returns 'unknown' on missing cache file (was 'free')
for the same reason: missing detection != confirmed Free instance
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 `@tests/setup/globalSetup.js`:
- Around line 59-68: fs.rmSync with maxRetries/retryDelay is a no-op unless
recursive:true, so replace the single fs.rmSync(tierFile, { force: true,
maxRetries: 2, retryDelay: 50 }) call with an explicit retry loop that attempts
to remove tierFile up to N times (e.g., 3), catching errors and, on
EPERM/EACCES, waiting retryDelay ms between attempts before retrying; use
fs.rmSync(tierFile, { force: true }) inside the loop, break on success, and on
final failure log the same warning using the existing error-to-string logic.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ec8f469d-6314-43a6-b2be-f952196cca6f
📒 Files selected for processing (2)
tests/setup/globalSetup.jstests/setup/tierGate.ts
…off loop fs.rmSync maxRetries/retryDelay are ignored unless recursive:true (Node docs). The previous call advertised retries that never fired. Replaced with an explicit 3-attempt loop with 50ms backoff so transient EPERM / EACCES locks on Windows actually get the intended retry window.
… convention Aligns globalSetup's tier-detection request with src/utils/fetch.ts: OAuth mode (OAUTH_ENABLED=true) → Authorization: Bearer PAT mode (default) → PRIVATE-TOKEN: <token> PATs accept both headers on GitLab, but PRIVATE-TOKEN is the canonical PAT header and matches the rest of the codebase — reduces confusion when debugging auth issues and avoids divergence from the project's own conventions.
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 `@tests/setup/globalSetup.js`:
- Line 91: The ESLint no-undef false positive for the Node global
AbortController (used where const controller = new AbortController(); is
declared) can be silenced by declaring the global at the top of the file; add a
file-level directive like /* global AbortController */ to
tests/setup/globalSetup.js (or alternatively add AbortController to your ESLint
globals or enable env: { node: true } in your ESLint config) so the linter
recognizes AbortController as defined.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c239c943-4c16-4327-b2c5-571f8fc233e0
📒 Files selected for processing (1)
tests/setup/globalSetup.js
…config
yarn lint previously matched only **/*.ts, so .js files (globalSetup.js,
sequencer.js, globalTeardown.js, etc.) escaped local lint and CI lint
even though they had real no-undef errors. CodeRabbit's independent
ESLint run caught it.
- Extend lint and lint:fix globs to {ts,js}
- Add AbortController, clearTimeout, clearInterval, setInterval,
Promise, URL, Buffer to the .js globals block in eslint.config.mjs
(previously had only setTimeout/fetch from the Node async surface)
|
Test Coverage ReportOverall Coverage: 96.6%
|
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
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
Integration suites that exercise Premium/Ultimate-only GitLab features now report as pending (
○ skipped) instead of failed (× failed) when the target instance is on the Free tier (or is an EE binary without an active license).Changes
tests/setup/tierGate.ts— new helper exposingdescribeIfTier/itIfTier/getDetectedTier/tierSatisfies. Skip labels include the required and detected tier for clear diagnostics.tests/setup/globalSetup.js— one-timecurrentLicenseGraphQL query at session start, result persisted toos.tmpdir()/gitlab-mcp-detected-tier-<repo-hash>.jsonso each Jest worker can read it synchronously at test parse time. Filename is namespaced by a hash of the checkout root so concurrent runs across worktrees (or NFS-shared tmpdirs) cannot collide. Detection catch path formats errors defensively (err instanceof Error ? err.message : String(err)).tests/integration/requirements.test.ts— wrapped withdescribeIfTier('ultimate', ...)(VERIFICATION_STATUSwidget)tests/integration/debug-widget-assignment.test.ts— wrapped withdescribeIfTier('premium', ...)(Epics + colour widget)tests/integration/data-lifecycle.test.ts— four Epic-dependent blocks gated withitIfTier('premium', ...)/describeIfTier('premium', ...). Non-tier-gated steps (group/project/MR/todos/etc.) still run on all tiers.Rationale
Tier-mismatch is an environment limitation, not a code defect. Hard-failing a test for a feature the licence prevents from running adds noise, hides real regressions, and forces developers to keep mental state about which failures are expected. Jest
it.skip/describe.skipwith a descriptive label is the idiomatic representation.Test plan
yarn test:allon a Free / unlicensed instance: 0 failed (previously 7 failed), 2 suites + 13 tests now report as skippedyarn test:allon an Ultimate instance: all tier-gated suites execute and pass (no behavioural change versus current main)yarn lintcleanyarn test(unit) unaffectedCloses #428