Skip to content

DRIVERS-3206 Address flakiness in command logging, retryable reads, and retryable writes unified tests for retryable handshake errors - #1988

Open
NoahStapp wants to merge 3 commits into
mongodb:masterfrom
NoahStapp:DRIVERS-3206
Open

NoahStapp wants to merge 3 commits into
mongodb:masterfrom
NoahStapp:DRIVERS-3206

Conversation

@NoahStapp

Copy link
Copy Markdown
Contributor

Add appName to each failpoint that deals with authenticated handshake failures in the three named test suites to prevent another background or racing operation from stealing the failpoint trigger.

Please complete the following before merging:

  • Is the relevant DRIVERS ticket in the PR title?
  • Update changelog.
  • Test changes in at least one language driver. Python.
  • Test these changes against all server versions and topologies (including standalone, replica set, and sharded
    clusters).

- commandStartedEvent
- commandSucceededEvent
- commandFailedEvent
uriOptions:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the beginning of this file it says Tests in this file are generated from handshakeError.yml.template.
Is it still true? If so, should we modify the template file as well?

id: &failPointClient failPointClient
useMultipleMongoses: false
uriOptions:
appName: "commandLoggingRedactionTest"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be setup on the main client? I think setting it up here will be valid only for the failpoint client.

data:
failCommands: [ping, saslContinue]
closeConnection: true
appName: "retryableReadHandshakeTest"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that the yml and json files are correct, but I think you need to fix the source .template file to add the appName. Otherwise when regenerating the yml files those should be lost.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

source/retryable-reads/tests/etc/templates/handshakeError.yml.template does have this change in the latest commit.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The template has the appName now, but only for the client. The template is missing the appName for the failpoint setup. From what I can see you should add it to line 75 and 127 in the reads template and 71 and 120 in writes template. Am I missing something?

@NoahStapp NoahStapp Oct 8, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I misunderstood your comment, fixed!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect, LGTMed 😄

@codeowners-service-app

Copy link
Copy Markdown

Assigned jyemin for team dbx-spec-maintainers-observability because comandeo-mongo is out of office.

@NoahStapp
NoahStapp requested a review from papafe October 8, 2026 13:30

@papafe papafe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants