test(NODE-7426): Add appName to OIDC test failpoints#4869
test(NODE-7426): Add appName to OIDC test failpoints#4869PavelSafronov wants to merge 5 commits intomongodb:mainfrom
Conversation
PavelSafronov
force-pushed
the
NODE-7426
branch
from
7cdb6ac to
553631d
Compare
dariakp
added
the
Primary Review
- configured a distinct per-test appName - removed generated appName
There was a problem hiding this comment.
Pull request overview
Updates OIDC authentication tests to scope server failpoints by appName (per updated OIDC spec guidance) and to explicitly clean up failpoints between test runs, reducing cross-test interference.
Changes:
- Add
serverless: forbidto the unified OIDC no-retry spec test. - Configure a stable
appNamein the unified spec test client and includeappNamein failpointdata. - Update OIDC prose integration tests to generate unique
appNamevalues per describe block, pass them to clients, and attempt explicit failpoint teardown.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 17 comments.
| File | Description |
|---|---|
| test/spec/auth/unified/mongodb-oidc-no-retry.yml | Adds serverless: forbid, introduces appName anchor, and scopes failpoints via appName. |
| test/spec/auth/unified/mongodb-oidc-no-retry.json | JSON equivalent of the unified spec updates (serverless forbid, appName, appName-scoped failpoints). |
| test/integration/auth/mongodb_oidc.prose.test.ts | Adds per-test appName generation, passes appName into clients and failpoints, and adds explicit failpoint cleanup (with issues noted in PR comments). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| client = getClient({ promoteValues: false, appName }, callbackSpy); | ||
| utilClient = getClient({ promoteValues: false, appName }, createCallback()); | ||
| collection = client.db('test').collection('test'); | ||
| await utilClient | ||
| .db() |
There was a problem hiding this comment.
The failpoint configured in this test should be scoped with appName to avoid impacting other OIDC prose tests. In this describe block, the client is created with an appName but the failCommand configuration later in the setup does not include appName in its data, so the failpoint can apply globally.
- typos fixed - appName added in spots it was missing
|
@tadjik1 FYI Copilot concerns addressed (most of the comments are wrong about appName), tests are passing. |
| client = getClient({}, callbackSpy); | ||
| utilClient = getClient({}, createCallback()); | ||
| client = getClient({ appName }, callbackSpy); | ||
| utilClient = getClient({ appName }, createCallback()); |
There was a problem hiding this comment.
I think we only want to set the appName on the client and not the utilClient, since the utilClient isn't the client under test. @nbbeeken can you check me on this?
There was a problem hiding this comment.
Yep, the appname is the thing the server is using to apply the failcommand or not, every connection to MongoDB starts with a hello that includes the client metadata (thus the appname). So that's how the failcommand gets filtered properly
There was a problem hiding this comment.
Makes sense, the test client must have the appName applied to it, while the util client needs to pass the appName to the various configureFailPoint commands. Updated.
dariakp
added
Team Review
Description
Summary of Changes
Updated OIDC test specs.
Double-checked that failpoints are being removed
Updated UnifiedMongoClient to always configure an
appName, as either a passed-in value or a unique identifier, as called out for OIDC tests in mongodb/specifications@4d11668Double check the following
npm run check:lint)type(NODE-xxxx)[!]: descriptionfeat(NODE-1234)!: rewriting everything in coffeescript