test: comprehensive unit + e2e coverage across SDK (stacked on #132)#133
Draft
zeevmoney wants to merge 8 commits into
Draft
test: comprehensive unit + e2e coverage across SDK (stacked on #132)#133zeevmoney wants to merge 8 commits into
zeevmoney wants to merge 8 commits into
Conversation
Add a shared mock seam (src/tests/helpers/mock-api.ts, createMockPermit) that patches the axios adapter on the REST, PDP and OPA transports and seeds API context without network, then add unit specs covering every API module (resources, roles, resource-roles, role-assignments, users, tenants, resource-instances, resource-relations, relationship-tuples, condition-sets, condition-set-rules, resource-actions/attributes/ action-groups, projects, environments, elements, deprecated), the enforcer (check/bulkCheck/getUserPermissions/checkAllTenants, string parsing, default-tenant, OPA path, response shaping, throwOnError) and the utils/config layer. Add one ABAC e2e (condition-sets) following the event-based, self-cleaning conventions. 262 new unit tests; full no-backend suite is 333 tests. Tests-only; no SDK source changes. Tests assert current behavior of two latent bugs (checkAllTenants payload PER-15318; unreachable PermitPDPStatusError), flagged in-code, not fixed here. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2 tasks
…rehensive-sdk-tests * per-15315/vitest-event-based-tests: ci: run unit/integration/module-imports on PR, defer e2e to next PR
…rehensive-sdk-tests * per-15315/vitest-event-based-tests: ci: run on all pull requests, not only those targeting main
…rehensive-sdk-tests * per-15315/vitest-event-based-tests: ci: pin PDP connection to IPv4 (127.0.0.1) in the backend test run
…rehensive-sdk-tests * per-15315/vitest-event-based-tests: test: make rbac useOpa checks opt-in and widen rebac CI budget
…rehensive-sdk-tests * per-15315/vitest-event-based-tests: test: poll the rbac multi-result reads to remove propagation races
A userset condition set referencing user.<attr> requires that attribute to exist on the built-in user resource; users.sync alone doesn't register it, so the condition-set creation failed with 400 MISSING_RESOURCE_ATTRIBUTE. Register a run-unique attribute on the __user resource before creating the userset, reference it consistently in the condition and the synced users, and remove it in the tolerant afterAll. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Condition sets compile to new policy (rego), which propagates slower than role/fact writes, so the 60s default left the ABAC check timing out in CI before the policy took effect. Match the heavier rebac budget. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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.
What kind of change does this PR introduce?
Test coverage. PR#3 of a 3-PR stack — stacked on test: migrate AVA→Vitest with event-based waits (stacked on #131) #132 (base branch
per-15315/vitest-event-based-tests, notmain). Tests-only — zero SDK source changes. Linear: PER-15317.What is the current behavior?
After the Vitest migration (test: migrate AVA→Vitest with event-based waits (stacked on #131) #132), the suite covers retry/config/wait-for-sync units, control-plane integration, and the RBAC/ReBAC/facts/bulk/lists e2e flows — but most individual API-module methods have no fast unit coverage, and ABAC (condition sets) has no e2e.
What is the new behavior?
Comprehensive unit + e2e coverage across the SDK:
src/tests/helpers/mock-api.ts—createMockPermit()installs a capturing axios adapter on all three transports (RESTconfig.axiosInstance, PDPenforcer.client, OPAenforcer.opaClient) and seeds API context with no network. The mock sits below all SDK logic, so the generated client's real path/param/body construction andhandleApiErrormapping execute for real — these tests catch regressions, they are not tautological.PermitApiError.check(string-resource parsing, default-tenant injection, OPA vs PDP path, response shaping),bulkCheck,getUserPermissions,checkAllTenants,throwOnError, and timeout passthrough.context,dict,regex,http-logger, andconfigenv parsing.e2e/condition-sets.e2e.spec.ts— the only significant flow with no prior e2e — following PR#2's conventions (unique-prefixed keys,waitForChecknot sleeps, complete tolerantafterAll, order-independent).Other information:
Tests
test:ci:fullon CI; not runnable locally without a backend)yarn test:ci:unit): 342 tests pass; lint clean.Two latent SDK bugs surfaced (characterization tests assert current behavior, clearly flagged in-code; not fixed here):
Enforcer.checkAllTenantssends{headers,params}as the POST body instead of axios config — tracked as PER-15318.Enforcer'sPermitPDPStatusErroris effectively unreachable: a trailing.catchre-wraps non-200 PDP responses asPermitConnectionError. Worth a follow-up fix + Linear issue.Deferred follow-ups (low value, noted by review)
bulkCheck/getUserPermissionsglobal-config.timeoutfallback (per-call timeout is tested);checkAllTenantserror path.Generated with Claude Code