Skip tests with dots and dollars in field names on 5.0+ #795
Conversation
| @@ -4016,7 +4016,7 @@ _test_insert_validate (insert_fn_t insert_fn) | |||
| } | |||
|
|
|||
| static void | |||
| test_insert_bulk_validate (void) | |||
| test_insert_bulk_validate (void *ctx) | |||
There was a problem hiding this comment.
These tests use error API 2, which means all server-side errors should be reported in the MONGOC_ERROR_SERVER domain. The expectations below use MONGOC_ERROR_COMMAND as the domain, which tells me these are client-side errors.
In that case, why should the server version have any impact on these tests? If anything, I would expect these tests to be removed entirely as part of CDRIVER-3895 since that will remove all client-side validation of dots and dollars. Alternatively, we could change the documents in these tests to violate some other validation rule that will be left in place (e.g. invalid UTF).
There was a problem hiding this comment.
This test fails with an assertion error: https://evergreen.mongodb.com/task_log_raw/mongo_c_driver_windows_2017_test_latest_sharded_noauth_nosasl_nossl_patch_d08ef111e1a9cf7e71d916f562b46a9d9bf061c6_60a625487742ae6f40858e18_21_05_20_09_01_04/0?type=T#L2378. You are correct that the first bulk insert expects a command error because it uses no insert validation. There is a second insert that specifies MONGOC_INSERT_NO_VALIDATE which expectedly fails on 5.0 and expects a server error.
I was considering rewriting the tests but in the interest of fixing the build so I can get on with the language refactoring (which has the potential of breaking lots of things) I decided to skip these tests for now and revisit them when we implement changes for handling field names with dots and dollars. I've added a TODO to re-enable the tests (also fixed clang-format while I was at it)
…801) * CDRIVER-3895: Update CRUD, transactions, and unified spec tests Also replaces CRUD legacy/v2 tests with unified equivalents for CDRIVER-3960. Synced with mongodb/specifications@a124e21 * Skip unified CRUD tests related to CDRIVER-3630 * Ensure upsertedIds is always present in BulkWriteResult * Allow array or document for updateOne/Many update arg * Remove dot/dollar restrictions for insert/replace validation This adds top-level key validation to _mongoc_validate_replace, similar to what existed in _mongoc_validate_update. * Remove redundant replace doc validation for legacy updates Replacement docs will already be validated by _mongoc_validate_replace before this function is reached. This adds logic to ensure "q" and "u" documents are present, similar to what existed in _mongoc_write_command_delete_legacy. * Test success and failure for legacy update code path The failure code path is testing that arguments are still validated by mongoc-util.c, despite replacement validation being removed in a previous commit. * Revert "Skip tests with dots and dollars in field names on 5.0+ (#795)" This reverts commit bd014ee. * Revise bulk and collection tests for allowing dots/dollars Empty keys can be used as a reliable BSON validation error, since insert, replace, and update all specify BSON_VALIDATE_EMPTY_KEYS in their vflags. Error message expectations are adjusted accordingly. An _id document with a dollar-prefixed key is also used as a reliable server-side validation error. * Allow methods in bulk_op_append to fail so expectError can be evaluated * Skip unified tests due to schema version and outstanding issues Allow entire files to be skipped to preempt schema version errors. * Ensure WC is set on database and collection entities Co-authored-by: Kevin Albertson <[email protected]>
…801) * CDRIVER-3895: Update CRUD, transactions, and unified spec tests Also replaces CRUD legacy/v2 tests with unified equivalents for CDRIVER-3960. Synced with mongodb/specifications@a124e21 * Skip unified CRUD tests related to CDRIVER-3630 * Ensure upsertedIds is always present in BulkWriteResult * Allow array or document for updateOne/Many update arg * Remove dot/dollar restrictions for insert/replace validation This adds top-level key validation to _mongoc_validate_replace, similar to what existed in _mongoc_validate_update. * Remove redundant replace doc validation for legacy updates Replacement docs will already be validated by _mongoc_validate_replace before this function is reached. This adds logic to ensure "q" and "u" documents are present, similar to what existed in _mongoc_write_command_delete_legacy. * Test success and failure for legacy update code path The failure code path is testing that arguments are still validated by mongoc-util.c, despite replacement validation being removed in a previous commit. * Revert "Skip tests with dots and dollars in field names on 5.0+ (#795)" This reverts commit bd014ee. * Revise bulk and collection tests for allowing dots/dollars Empty keys can be used as a reliable BSON validation error, since insert, replace, and update all specify BSON_VALIDATE_EMPTY_KEYS in their vflags. Error message expectations are adjusted accordingly. An _id document with a dollar-prefixed key is also used as a reliable server-side validation error. * Allow methods in bulk_op_append to fail so expectError can be evaluated * Skip unified tests due to schema version and outstanding issues Allow entire files to be skipped to preempt schema version errors. * Ensure WC is set on database and collection entities Co-authored-by: Kevin Albertson <[email protected]>
Follows #793. Versioned API tests are fixed by #792. With all patches applied, tests pass again: https://spruce.mongodb.com/version/60a62eabc9ec443716160e3d/tasks
Note that these tests will be replaced by spec tests after the completion of DRIVERS-1237 (see mongodb/specifications#980)