test(integration): authenticate Integration tests with Minter using user token - #670
test(integration): authenticate Integration tests with Minter using user token#670amrit-agarwal-1 wants to merge 7 commits into
Conversation
|
✅ No issues found. Checked for bugs and CLAUDE.md compliance. |
2 similar comments
|
✅ No issues found. Checked for bugs and CLAUDE.md compliance. |
|
✅ No issues found. Checked for bugs and CLAUDE.md compliance. |
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
Review summary One new finding: shell quoting in the Minter docker run command (see inline comment on Everything else looks good — the dual-auth cache keying, |
|
✅ No issues found. Checked for bugs and CLAUDE.md compliance. |
1 similar comment
|
✅ No issues found. Checked for bugs and CLAUDE.md compliance. |
|
Review summary One new finding: double-cleanup in the Everything else looks correct — the dual-key cache ( |
|
✅ No issues found. Checked for bugs and CLAUDE.md compliance. |
17c621e to
da239c9
Compare
|
Review summary One new finding: missing guard in the Everything else looks correct: the dual-key service cache ( |
|
✅ No issues found. Checked for bugs and CLAUDE.md compliance. |
37dd36d to
fe21be3
Compare
|
✅ No issues found. Checked for bugs and CLAUDE.md compliance. |
|
Review summary One new finding: null-dereference in the pageSize test in Three prior threads remain open (coverage.yml quoting, feedback double-cleanup, attachments afterAll guard) — no change to their status this run. |
|
Review summary One new finding: the second Three prior threads remain open (line 184 quoting, feedback double-cleanup, attachments afterAll guard, traces null dereference) — no change to their status this run. |
|
Review summary One new finding: orphaned JSDoc comment in Four prior threads remain open (coverage.yml shell quoting ×2, feedback double-cleanup, attachments |
|
✅ No issues found. Checked for bugs and CLAUDE.md compliance. |
4 similar comments
|
✅ No issues found. Checked for bugs and CLAUDE.md compliance. |
|
✅ No issues found. Checked for bugs and CLAUDE.md compliance. |
|
✅ No issues found. Checked for bugs and CLAUDE.md compliance. |
|
✅ No issues found. Checked for bugs and CLAUDE.md compliance. |
amrit-agarwal-1
changed the title
|
we are running vo mode with PAT then user token and again v1 mode with PAT and user all serially. This will double our integration tests run time. I dont see a way to run them parallel without redesigning the entire integration tests. I would suggest to increase the runner cpu from 2 to 4 and increase workercount to 4 which will increase the concurrence between different test suits and improve the time |
|
✅ No issues found. Checked for bugs and CLAUDE.md compliance. |
1 similar comment
|
✅ No issues found. Checked for bugs and CLAUDE.md compliance. |
4778f72 to
1e779ce
Compare
|
✅ No issues found. Checked for bugs and CLAUDE.md compliance. |
1 similar comment
|
✅ No issues found. Checked for bugs and CLAUDE.md compliance. |
| # Network-bound suite, but the tenant is the bottleneck, not the runner. | ||
| # 4 workers bought 13% wall clock (1292s -> 1120s) and cost a wave of 504s, | ||
| # 503s and request timeouts that tripled the failing files. Stay at 2. | ||
| run: npm run test:integration:coverage -- --run --maxWorkers=2 |
There was a problem hiding this comment.
There will be no improvement by increasing maxWorkers without increasing cpu. check if we can increase cpu to 4 from 2. Also keep the comment in 1 line or remove it all together
1e779ce to
5701539
Compare
|
✅ No issues found. Checked for bugs and CLAUDE.md compliance. |
1 similar comment
|
✅ No issues found. Checked for bugs and CLAUDE.md compliance. |
… under both Squashed from 25 commits (kept on backup/minter-pre-rebase). - Mint a user access token with Minter in CI (ACR scoped token, masked, written into tests/.env.integration) so suites whose APIs reject PATs can run at all: insightsrtm_ (Agents, Memory, Agent Traces, Governance) and the notification service, all previously describe.skip. - describeIntegration(name, requirement, modes, body, options?) declares a suite once and expands it over init modes x credentials, naming cells [initMode][authMode]. 'any' runs under every configured credential — PAT and user token both, per the decision that neither subsumes the other. - User-token cells resolve MINTER_BASE_URL when set; PAT cells keep UIPATH_BASE_URL. getActiveAuth() gives the raw-fetch call sites the credential and host of the cell actually running. - Fixture ownership: suites create and own what they read rather than depending on tenant state; rolling time windows replace hardcoded dates; the traces suite falls back to expired spans when the pinned trace ages out. - Five tests skipped with the blocker named: the notification $filter server stall, topic groups absent from the tenant, and three agent-activity data gaps. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…mments 'any' read as 'either one will do' when it means 'run under every configured credential' — the suite executes once per credential, not once. 'both' says that. Type, call sites, and docs renamed together; no behaviour change. Also trimmed the comment blocks that had grown past their value, including a JSDoc citing a specific CI run number that would not age well. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…drop unused helpers - Unskip the 18 insightsrtm_ describe blocks in maestro processes/cases/case-instances and gate them on the user-token cell instead; they were skipped only because PAT is rejected there. - Remove the unused canAuthenticate() helper. - Make initializeServices' authMode required — every caller passes it. - Delete tests/.env.integration after the run so the minted token does not outlive the job on a reused runner.
The env var the tests read is now UIPATH_TENANT_ID. The GitHub secret keeps the UIPATH_TENANT_ID_DEV || UIPATH_TENANT_ID override pair, matching every other secret in the workflow.
15 of the 18 unskipped insightsrtm_ blocks pass under the user token. The three that do not fail on tenant data, not auth: getInstanceStats' duration fields are undefined with no completed instances in the window, and getSlaSummary's slaDueTime is empty. Also drops the worker-count comment on the integration step, per review.
d4f2bca to
b3991cb
Compare
|
✅ No issues found. Checked for bugs and CLAUDE.md compliance. |
2 similar comments
|
✅ No issues found. Checked for bugs and CLAUDE.md compliance. |
|
✅ No issues found. Checked for bugs and CLAUDE.md compliance. |
285d4ae to
b3991cb
Compare
|
✅ No issues found. Checked for bugs and CLAUDE.md compliance. |
The pat and user cells ran back-to-back in one job, so the run cost the sum of both credentials. Each leg now runs on its own runner via INTEGRATION_AUTH_MODE, which needs SonarCloud out of the test job: it must see both lcov reports and must run once, so it moves to a job that downloads and merges them.
|
✅ No issues found. Checked for bugs and CLAUDE.md compliance. |
Its reads stall past the default 30s on the shared tenant — 18 timeouts in the last run, spread across the whole file. Same class and remedy as the schema suite's hooks in #723: budget headroom, not failure tolerance.
|
✅ No issues found. Checked for bugs and CLAUDE.md compliance. |
3 participants
Six suites - Agents, Agent Memory, Agent Traces, Governance, Notifications, Subscriptions , were permanently describe.skip-ed: their APIs reject PAT and client-credentials tokens with 401 regardless of scopes, and the required scopes aren't available to external applications.
CI now mints a user access token with Minter (Portal team's headless-login image, pulled from ACR with a scoped token) and writes it to UIPATH_USER_TOKEN. Those six suites run under it.
Suites are declared with describeIntegration(name, requirement, modes, body), which expands them over init modes × credentials and names each cell [initMode][authMode], so a failure says which credential failed. The requirement is stated once and drives the credential, the host and the collection-time skip guard — declaring the guard separately let the two disagree.
'both' — the default — runs a suite once per configured credential. 23 suites run under PAT and the user token: the PAT exercises the external-application scope model most SDK consumers use, the user token exercises the API surface generally. Neither subsumes the other.
User-token cells use MINTER_BASE_URL (the platform host). The default host is a CORS proxy whose path whitelist must name every service a suite touches.
Also in here:
getActiveAuth() gives the four raw-fetch call sites the credential and host of the cell actually running; config.userToken ?? config.secret was correct only while a suite ran under one credential.
Folder-filter tests use INTEGRATION_TEST_FOLDER_KEY instead of fabricated GUIDs — folderKeys is authorized before it's applied, so an inaccessible folder returns 403, an assumption these tests documented but had never run to verify.
Fixtures are created and owned by the suites that read them.
Five tests are skipped with the blocker named: a notification-service $filter stall, topic groups absent from the tenant, and three agent-activity data gaps.
Local runs are unchanged — with only UIPATH_SECRET, 'both' suites run PAT-only and user suites skip. Minter stays optional.
Verified against alpha procodeapps/integrationtest.