Skip to content

DRIVERS-2294 Add createCollection and collMod tests for changeStreamPreAndPostImages #1206

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 5, 2022
Merged

DRIVERS-2294 Add createCollection and collMod tests for changeStreamPreAndPostImages #1206

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
abaf933
DRIVERS-2294 Add createCollection and collMod tests for CS images
May 3, 2022
4d47485
Remove outdated statement on schema version
May 3, 2022
a32ac9a
Jeremy comments
May 3, 2022
a6cb421
Split tests into two files
May 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions source/collection-management/tests/README.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,4 @@ Collection Management Tests
===========================

This directory contains tests for collection management. They are implemented
in the `Unified Test Format <../../unified-test-format/unified-test-format.rst>`__
and require schema version 1.0.
in the `Unified Test Format <../../unified-test-format/unified-test-format.rst>`__.
173 changes: 173 additions & 0 deletions source/collection-management/tests/pre_and_post_images-collection.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,173 @@
{
"description": "pre_and_post_images-collection",
"schemaVersion": "1.9",
"runOnRequirements": [
{
"minServerVersion": "6.0"
}
],
"createEntities": [
{
"client": {
"id": "client0",
"observeEvents": [
"commandStartedEvent"
]
}
},
{
"database": {
"id": "database0",
"client": "client0",
"databaseName": "papi-tests"
}
},
{
"collection": {
"id": "collection0",
"database": "database0",
"collectionName": "test"
}
}
],
"initialData": [
{
"collectionName": "test",
"databaseName": "papi-tests",
"documents": []
}
],
"tests": [
{
"description": "createCollection with changeStreamPreAndPostImages enabled",
"operations": [
{
"name": "dropCollection",
"object": "database0",
"arguments": {
"collection": "test"
}
},
{
"name": "createCollection",
"object": "database0",
"arguments": {
"collection": "test",
"changeStreamPreAndPostImages": {
"enabled": true
}
}
},
{
"name": "assertCollectionExists",
"object": "testRunner",
"arguments": {
"databaseName": "papi-tests",
"collectionName": "test"
}
}
],
"expectEvents": [
{
"client": "client0",
"events": [
{
"commandStartedEvent": {
"command": {
"drop": "test"
},
"databaseName": "papi-tests"
}
},
{
"commandStartedEvent": {
"command": {
"create": "test",
"changeStreamPreAndPostImages": {
"enabled": true
}
},
"databaseName": "papi-tests"
}
}
]
}
]
},
{
"description": "modifyCollection to changeStreamPreAndPostImages enabled",
"operations": [
{
"name": "dropCollection",
"object": "database0",
"arguments": {
"collection": "test"
}
},
{
"name": "createCollection",
"object": "database0",
"arguments": {
"collection": "test",
"changeStreamPreAndPostImages": {
"enabled": false
}
}
},
{
"name": "assertCollectionExists",
"object": "testRunner",
"arguments": {
"databaseName": "papi-tests",
"collectionName": "test"
}
},
{
"name": "modifyCollection",
"object": "database0",
"arguments": {
"collection": "test",
"changeStreamPreAndPostImages": {
"enabled": true
}
}
}
],
"expectEvents": [
{
"client": "client0",
"events": [
{
"commandStartedEvent": {
"command": {
"drop": "test"
},
"databaseName": "papi-tests"
}
},
{
"commandStartedEvent": {
"command": {
"create": "test",
"changeStreamPreAndPostImages": {
"enabled": false
}
}
}
},
{
"commandStartedEvent": {
"command": {
"collMod": "test",
"changeStreamPreAndPostImages": {
"enabled": true
}
}
}
}
]
}
]
}
]
}
87 changes: 87 additions & 0 deletions source/collection-management/tests/pre_and_post_images-collection.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
description: "pre_and_post_images-collection"

schemaVersion: "1.9"

runOnRequirements:
- minServerVersion: "6.0"

createEntities:
- client:
id: &client0 client0
observeEvents: [ commandStartedEvent ]
- database:
id: &database0 database0
client: *client0
databaseName: &database0Name papi-tests
- collection:
id: &collection0 collection0
database: *database0
collectionName: &collection0Name test

initialData:
- collectionName: *collection0Name
databaseName: *database0Name
documents: []

tests:
- description: "createCollection with changeStreamPreAndPostImages enabled"
operations:
- name: dropCollection
object: *database0
arguments:
collection: *collection0Name
- name: createCollection
object: *database0
arguments:
collection: *collection0Name
changeStreamPreAndPostImages: { enabled: true }
- name: assertCollectionExists
object: testRunner
arguments:
databaseName: *database0Name
collectionName: *collection0Name
expectEvents:
- client: *client0
events:
- commandStartedEvent: &dropEvent
command:
drop: *collection0Name
databaseName: *database0Name
- commandStartedEvent:
command:
create: *collection0Name
changeStreamPreAndPostImages: { enabled: true }
databaseName: *database0Name
- description: "modifyCollection to changeStreamPreAndPostImages enabled"
operations:
- name: dropCollection
object: *database0
arguments:
collection: *collection0Name
- name: createCollection
object: *database0
arguments:
collection: *collection0Name
changeStreamPreAndPostImages: { enabled: false }
- name: assertCollectionExists
object: testRunner
arguments:
databaseName: *database0Name
collectionName: *collection0Name
- name: modifyCollection
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The Go driver has no modifyCollection helper, so if someone else could POC this test that'd be great 🙇

Copy link
Contributor

Choose a reason for hiding this comment

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

Do any drivers actually have collMod helpers? (Swift and Rust do not)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jmikola It sounds like it might just be PHP. Any thoughts on making this an integration test within the PHP driver?

Copy link
Member

Choose a reason for hiding this comment

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

If it's not too much trouble, I think leaving it in place and having drivers skip it would be a good exercise for ensuring each driver has some mechanism to skip unified spec tests as needed.

But no objections if you'd rather remove this. I can test in PHP easily.

Copy link
Contributor

Choose a reason for hiding this comment

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

I’m not in general opposed to requiring drivers update their runners to be able to easily skip these kinds of tests if they can’t already. But I don’t know if we should bundle that with required 6.0 changes everyone is on a tight timeline for. I’d be more inclined to make the skipping capability a unified test runner requirement and add sample tests drivers need to skip to prove they can do it, or something like that.

If we do leave it, could we at least put it in a separate file? In Swift (and I believe also Rust) we error while decoding a JSON file into our native test representation type if any operation names are unrecognized, since we have a concrete type modeling each recognized operation.
We can easily skip decoding a file altogether. But currently to skip a test with an unsupported operation within a file of tests we otherwise want to run, I think we’d need to do something like add dummy modifyCollection operations to allow decoding to succeed, or do some kind of larger refactor to more gracefully support this type of thing.

Copy link
Member

Choose a reason for hiding this comment

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

Given that, I propose we split these tests into two files:

  • createCollection-pre_and_post_images
  • modifyCollection-pre_and_post_images

Then, drivers can skip as needed.

Additionally, I asked @benjirewis to add a DRIVERS ticket to add a valid-fail test for operation-unsupported (similar to operation-failure, except that test runs a non-existent server command).

@kmahar: Separately, I think you'll want to address the decoding issue in Swift/Rust ASAP. @benjirewis mentioned that the CSOT tests will likely include a whole bunch of operations in one file (some likely unimplemented in Swift). If you think this warrants a DRIVERS ticket first to require all drivers be capable of selective skipping (vs. entire files), feel free to mandate the work through that -- I think it'd be easy for already-compliant drivers to mark downstream tickets as "works as designed" if needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Split into two different files and created DRIVERS-2313 to add a valid-fail test for unsupported operations.

Copy link
Member

@ShaneHarvey ShaneHarvey May 4, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

FWIW, the python test runner will skip tests that use helpers pymongo does not support. For example:

        # Some tests need to be skipped based on the operations they try to run.
        for op in spec["operations"]:
            name = op["name"]
            if name == "count":
                self.skipTest("PyMongo does not support count()")
            if name == "listIndexNames":
                self.skipTest("PyMongo does not support list_index_names()")
            if client_context.storage_engine == "mmapv1":
                if name == "createChangeStream":
                    self.skipTest("MMAPv1 does not support change streams")
                if name == "withTransaction" or name == "startTransaction":
                    self.skipTest("MMAPv1 does not support document-level locking")

When we sync these tests we would just need to add "modifyCollection" to the skip condition which I'm fine with.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Go does not yet have the logic you've described above for Python, but I'll add it as part of GODRIVER-2396. We currently have a global list of tests that need to be skipped, which I've been adding to locally in recent changes that have tests with unsupported operations.

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks for splitting the tests; and good to know @jmikola, I think we'll be able to add some kind of workaround for that without a ton of trouble

object: *database0
arguments:
collection: *collection0Name
changeStreamPreAndPostImages: { enabled: true }
expectEvents:
- client: *client0
events:
- commandStartedEvent: *dropEvent
- commandStartedEvent:
command:
create: *collection0Name
changeStreamPreAndPostImages: { enabled: false }
- commandStartedEvent:
command:
collMod: *collection0Name
changeStreamPreAndPostImages: { enabled: true }