Repository navigation
Conversation
The explain queryShapeHash / planCacheShapeHash tests pulled the hash out of the explain result in local helpers and raised RuntimeError when the field was missing or explain failed. That failure happened before any assertion helper ran, so an engine that does not report these fields failed every case with an exception instead of an assertion result. Replace the four helpers with explain_find / _explain_aggregate, which only return the raw explain result, and a shared assert_hash_relation helper in diagnostic/utils/explain_hash.py. It checks in a single assertProperties call that both hashes are non-empty strings and that they are equal (same shape) or different (different shape). An explain error is passed to assertProperties and fails there with the server error. Behavior change: a different-shape test now fails when the second explain has no hash, where before Ne passed on a missing field. Test expectations are otherwise unchanged; 168/168 still pass on MongoDB 8.2.4. Signed-off-by: Samar Farooqi <samafarc@amazon.com>
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.
Assert explain hash fields through assertProperties
Problem
The explain hash tests (
test_explain_queryhash_shape.py,test_explain_queryhash_pipeline.py) readplanCacheShapeHash/queryShapeHashwith local helpers that raiseRuntimeErrorwhen explain fails or the field is missing. The error fires before any assertion runs. On an engine that doesn't report these hashes, all 168 cases fail with a helper error instead of an assertion failure.Changes
system/diagnostic/utils/explain_hash.pywithassert_hash_relation(result_a, result_b, paths, same, msg). In oneassertPropertiescall it checks:same=True) or different (same=False)assertProperties, which reports the server error as an assertion failure.get_*/_get_*helpers are replaced byexplain_find/_explain_aggregate, which return the raw explain result.$out/$mergelayout (stages.0.$cursor.queryPlanner.planCacheShapeHash) is kept as a second candidate path.Expectations are unchanged. One check is stricter: a
Necomparison no longer passes when a hash is missing.Testing
assertProperties, with the same signature across two runs, instead of raisingRuntimeError.$out/$mergepath, explain error.Example failure on an engine without the field: