Report $out and planCacheSetFilter failures through the assertion by samafarc · Pull Request #729 · documentdb/functional-tests · GitHub
Skip to content

Report $out and planCacheSetFilter failures through the assertion - #729

Open
samafarc wants to merge 2 commits into
documentdb:mainfrom
samafarc:investigate-out-geo-filters
Open

samafarc wants to merge 2 commits into
documentdb:mainfrom
samafarc:investigate-out-geo-filters

Conversation

@samafarc

Copy link
Copy Markdown
Contributor
  • $out tests read the target collection through listCollections and index into the result. When the engine rejects the $out, for example $out.timeseries, the test crashes with IndexError. It now checks the $out result first and fails at assertSuccess with the engine's error.
  • planCacheSetFilter tests read indexFilterSet from explain with []. An engine that omits the field crashes with KeyError. The tests now use .get() and fail at assertProperties.

No behavior change on MongoDB. On other engines, no test switches from failing to passing. The crash just becomes an assertion failure.

Testing: MongoDB 8.2.4, changed files only. The 2 $out files: 57 passed. The 3 planCacheSetFilter files: 24 passed.

Signed-off-by: Samar Farooqi <samafarc@amazon.com>
…lter tests

Signed-off-by: Samar Farooqi <samafarc@amazon.com>
@samafarc
samafarc requested a review from a team as a code owner October 19, 2026 20:20

@eerxuan eerxuan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Summary

Turns two crash-on-unsupported patterns into assertion failures: the $out tests index
into listCollections after a failed $out, and the planCacheSetFilter tests subscript
indexFilterSet. I checked the $out change with a stubbed executor that makes $out
fail: main raises IndexError, this branch raises an AssertionError carrying the
engine's error. CI is green on both mongo targets. One file with the same $out pattern
is not covered (item 1).

Critical Issues

  1. [Required] test_out_write_behavior.py:100 has the same crash and is not changed.
    test_out_database_name_acceptance calls $out, discards the result, then reads
    cast(dict, result)["cursor"]["firstBatch"][0] from listCollections. An engine that
    rejects $out still gets an IndexError there. It is the third file with this pattern
    (grep 'firstBatch"\]\[0\]' in stages/out/ finds exactly these three). Please apply
    the same fix, or say why this file is excluded.

Positive Feedback

  • The root cause is correctly identified: the crash hides the engine's actual error.
  • The scope is small, and the description says what was run (57 and 24 passed on MongoDB 8.2.4).
  • .get() for indexFilterSet is correct for the assertion. A missing field yields None,
    which Eq(True) rejects, so no test can switch from failing to passing.

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.

2 participants