Add fsync command tests

[danielfrankcom]

This change adds tests for the fsync command.

There is some machinery overlap with #641 but they have the same content so they should merge cleanly.

Closes #420

[documentdb-triage-tool]

🤖 Auto-triaged by documentdb-triage-tool.

Applied: compatibility test, enhancement
Project fields suggested: Component test-coverage · Priority P2 · Effort L · Status Needs Review
Confidence: 0.30 (deterministic)

Reasoning

component from path globs (test-coverage, test-framework); effort from diff stats (1081+1 LOC, 11 files); LLM failed: Invalid response body while trying to fetch https://api.anthropic.com/v1/messages: Premature close

If a label is wrong, remove it manually and ping @patty-chow so the rules can be tuned. The bot will not re-label items that already have component labels.

[ramgokul-14]

Required - Doc update needed for conftest.py validator allowance — the change to test_structure_validator.py is a project-wide convention change. Per REVIEW.md §9, update TEST_FORMAT.md or FOLDER_STRUCTURE.md to document that conftest.py is a valid non-test Python file.

Suggestions

• Add a max-iteration guard to drain_fsync_lock's while True to prevent CI hangs
• Assert the first lock acquisition succeeded in test_fsync_lock_persists_after_connection_close before closing the client
• Consider an inline comment in test_fsync_rejects_non_admin_database explaining why execute_command (non-admin) is used

Questions

• Is afterClusterTime in readConcern in-scope, or is the current level-focused testing sufficient for an admin command that mostly rejects these options?
• Was placing the fixture at administration/conftest.py (parent level) considered vs. the re-export pattern?

[danielfrankcom]

Required - Doc update needed for conftest.py validator allowance — the change to test_structure_validator.py is a project-wide convention change. Per REVIEW.md §9, update TEST_FORMAT.md or FOLDER_STRUCTURE.md to document that conftest.py is a valid non-test Python file.

Added a note about this.

• Add a max-iteration guard to drain_fsync_lock's while True to prevent CI hangs

I don't think this is needed. If the loop does not progress and reach a guard condition then there is a critical bug in the engine itself, which could apply to any command we execute hanging forever in the engine. The CI will eventually time out and fail the test run regardless of the problem with the engine so I don't think this is a concern we need to worry about.

• Assert the first lock acquisition succeeded in test_fsync_lock_persists_after_connection_close before closing the client

The final lockCount 2 assertion already proves the first acquire succeeded, since a failed one would leave the count at 1. The test framework also encourages only 1 assertion per method so it is easier this way.

• Consider an inline comment in test_fsync_rejects_non_admin_database explaining why execute_command (non-admin) is used

I think the property comment and test method naming already make this clear enough.

• Is afterClusterTime in readConcern in-scope, or is the current level-focused testing sufficient for an admin command that mostly rejects these options?

Added tests for both missing readConcern sub-fields. afterClusterTime and atClusterTime are now covered.

• Was placing the fixture at administration/conftest.py (parent level) considered vs. the re-export pattern?

The re-export keeps the autouse lock-drain scoped to just the fsync and fsyncUnlock directories. A parent conftest.py under administration/ would run the drain around every admin test, most of which have nothing to do with the fsync lock.