Make smoke tests capability probes; fix 5 that crash on engine divergence - #718
Open
eerxuan wants to merge 1 commit into
Open
Make smoke tests capability probes; fix 5 that crash on engine divergence#718eerxuan wants to merge 1 commit into
eerxuan wants to merge 1 commit into
Conversation
…ence
Smoke tests are meant to answer "does the engine support this feature?" — one
minimal probe per feature. Several were written as full round-trip assertions,
so they failed on cosmetic divergences (e.g. `ok` returned as int32 vs the
double the expected value used) even though the feature works, or crashed with
unhelpful TypeError/IndexError when a setup step behaved differently.
framework/assertions.py:
- assertCommandSupported(result): passes if the command returned a result
rather than erroring; no value/BSON-type comparison. The capability probe.
- assertChangeStreamEvent(result): command succeeded and cursor.nextBatch is
non-empty (an event was emitted); no comparison of event contents.
Fixes (5 that errored on non-matching behaviour):
- getMore / killCursors: extract the cursor id defensively so a failed setup
find reports cleanly through the single assertion instead of raising
"'OperationFailure' object is not subscriptable".
- changeStream createIndexes / dropIndexes / modify: extract the cursor id
defensively and assert an event was emitted, instead of indexing
nextBatch[0] into an IndexError.
Reduced to capability probes (25): index create/properties/types, minN/maxN
accumulator+expression+window, $rank, planCacheListFilters, and the collStats/
connectionStatus/dataSize/dbStats/hostInfo/logout diagnostics. These now assert
only that the command is supported; exact response shape stays the job of each
feature's detailed tests.
Each test keeps exactly one assertion (test-format validator compliant).
Not changed here: the assert-on-error smoke tests (commitTransaction,
abortTransaction, compactStructuredEncryptionData, setIndexCommitQuorum,
authenticate). Asserting a specific error is a poor support signal and can't be
special-cased per engine; converting them to happy-case probes is a follow-up.
Signed-off-by: Yunxuan Shi <yunxuan@amazon.com>
|
🤖 Auto-triaged by documentdb-triage-tool. Applied: Reasoningcomponent from path globs (test-coverage, test-framework); P0 keyword "crash" in title; effort from diff stats (129+175 LOC, 31 files); LLM: Adds new assertion helpers and refactors ~30 smoke tests to be defensive capability probes, touching framework/assertions.py and multiple test files across one component. If a label is wrong, remove it manually and ping |
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
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.
Summary
Smoke tests are meant to answer "does the engine support this feature?" — one minimal probe per feature. Several were written as full round-trip assertions, so they failed on cosmetic divergences (e.g.
okreturned as int32 vs the double the expected value used) even though the feature works, or crashed with unhelpfulTypeError/IndexErrorwhen a setup step behaved differently.Framework (
framework/assertions.py)assertCommandSupported(result)— passes if the command returned a result rather than erroring; no value/BSON-type comparison. The capability probe.assertChangeStreamEvent(result)— command succeeded andcursor.nextBatchis non-empty (an event was emitted); no comparison of event contents.Fixes (5 that errored on non-matching behaviour)
findreports cleanly through the single assertion instead of raising'OperationFailure' object is not subscriptable.nextBatch[0]into anIndexError.Reduced to capability probes (25)
index create/properties/types, minN/maxN accumulator+expression+window,
$rank, planCacheListFilters, and the collStats/connectionStatus/dataSize/dbStats/hostInfo/logout diagnostics. These now assert only that the command is supported; exact response shape stays the job of each feature's detailed tests.Each test keeps exactly one assertion (test-format validator compliant).
Not changed here
The assert-on-error smoke tests (commitTransaction, abortTransaction, compactStructuredEncryptionData, setIndexCommitQuorum, authenticate). Asserting a specific error is a poor support signal and can't be special-cased per engine; converting them to happy-case probes is a follow-up.
Testing
Verified against a live engine: 24 non-replica-set fixed smoke tests pass; the 3
$setWindowFieldsones pass with the window-fields feature flag on; the 3 changeStream ones arereplica_set-gated.