PYTHON-2033 Unified Test Format #519

prashantmital · 2020-11-17T09:11:29Z

PYTHON-2033

To Do:

Implement $$matchesEntity operator
~~Implement $$matchesHexBytes operator~~ - this will be implemented along with GridFS since it is not used by any tests other than GridFS.
Implement support for expectError.errorContains
Implement support for expectError.errorCode
Implement support for expectError.errorCodeName
Implement targetedFailPoint operation
Add support for uriOptions and useMultipleMongoses to MongoClient entity creation
Add support for all BSON types to $$type operator

test/test_unified_format.py

test/unified_format.py

test/test_unified_format.py

ShaneHarvey

Looks good so far!

test/test_unified_format.py

test/__init__.py

ShaneHarvey · 2020-12-08T19:54:12Z

test/utils.py

@@ -948,3 +956,118 @@ def assertion_context(msg):
    except AssertionError as exc:
        msg = '%s (%s)' % (exc, msg)
        py3compat.reraise(type(exc), msg, sys.exc_info()[2])
+
+
+class SpecParserUtil(object):


Is this class needed?

SpecParserUtil is still here.

Sorry, removed.

test/unified_format.py

prashantmital · 2020-12-10T21:49:13Z

Implementation of $$matchesHexBytes will be done as part of PYTHON-2459.

ShaneHarvey · 2020-12-15T21:50:15Z

pymongo/helpers.py

@@ -224,7 +224,6 @@ def _check_write_command_response(result):
    write_errors = result.get("writeErrors")
    if write_errors:
        _raise_last_write_error(write_errors)
-


We can remove this change now.

ShaneHarvey · 2020-12-15T22:18:16Z

test/unified_format.py

+        # process test-level runOnRequirements
+        run_on_spec = spec.get('runOnRequirements', [])
+        if not self.should_run_on(run_on_spec):
+            raise unittest.SkipTest('runOnRequirements not satisfied')


Shouldn't runOnRequirements happen first, ie before processing createEntities and initialData?

good point. Also moved up the processing of skipTest.

ShaneHarvey · 2020-12-15T22:19:22Z

test/utils.py

@@ -948,3 +956,118 @@ def assertion_context(msg):
    except AssertionError as exc:
        msg = '%s (%s)' % (exc, msg)
        py3compat.reraise(type(exc), msg, sys.exc_info()[2])
+
+
+class SpecParserUtil(object):


SpecParserUtil is still here.

ShaneHarvey · 2020-12-15T22:21:32Z

test/unified_format.py

+                self._listeners[spec['id']] = listener
+                kwargs['event_listeners'] = [listener]
+            if client_context.is_mongos and spec.get('useMultipleMongoses'):
+                kwargs['host'] = client_context.mongos_seeds()


Change this to kwargs['h'] to fix the host errors. rs_or_single_client accepts a h argument, not host.

prashantmital · 2020-12-16T00:08:18Z

In 1a79309 I made one more change to how we were skipping entire classes when runOn wasn't satisfied. Previously, we got this extremely unhelpful message when skipTest was invoked in setUpClass:

skipped 'runOnRequirements not satisfied'

With my change, this is now more informative:

skipped 'TestUnifiedTestFormatValidPassPocTransactionsMongosPinAuto runOnRequirements not satisfied'

The other change tweaks how we deal with unsupported schema versions - this was a bug.

ShaneHarvey · 2020-12-16T00:11:30Z

test/unified_format.py

+                if mixin_class is None:
+                    print('Ignoring test file %s with '
+                          'unsupported schemaVersion %s' %
+                          (fpath, schema_version))


Shouldn't we raise some kind of error here so that we actually notice when this happens?

If we do that, we cannot get the test suite green with the valid-fail tests: https://github.com/mongodb/specifications/tree/master/source/unified-test-format/tests

Shouldn't the valid-fail tests catch said error to assert that the tests actually do fail?

Like if we copy/paste this test file https://github.com/mongodb/specifications/blob/master/source/unified-test-format/tests/valid-fail/schemaVersion-unsupported.yml into the valid-pass directory, and then run the test suite, it should fail. What's the behavior in that scenario right now?

I would think if we expect something to fail, we would mark it as an expected failure and have the test suite still pass. This is what we are doing at the moment with the valid-fail tests (see the expected_failures argument):

globals().update(generate_test_classes( os.path.join(_TEST_PATH, 'valid-fail'), module=__name__, class_name_prefix='UnifiedTestFormat', expected_failures=[ 'foo', 'FindOneAndReplace returnDocument invalid enum value', 'FindOneAndUpdate returnDocument invalid enum value' ]))

IIRC the spec is not prescriptive for this. We need to decide what the correct behavior should be.

I don't think we should need to list out the expected_failures. I think it's clear that if a test is in the valid-fail directory it should fail and any test in valid-pass should pass. Is that the question? Or is the question what type of error is expected?

I'd like to avoid squashing errors and instead raise them.

Updated the code to raise errors. The unsupported schema test file now causes a failure during test class generation as follows:

(pymongo_cp37) ➜ mongo-python-driver git:(PYTHON-2033/unified-test-format) ✗ python setup.py test -s test.test_unified_format running test running egg_info writing pymongo.egg-info/PKG-INFO writing dependency_links to pymongo.egg-info/dependency_links.txt writing requirements to pymongo.egg-info/requires.txt writing top-level names to pymongo.egg-info/top_level.txt reading manifest file 'pymongo.egg-info/SOURCES.txt' reading manifest template 'MANIFEST.in' writing manifest file 'pymongo.egg-info/SOURCES.txt' running build_ext Traceback (most recent call last): File "setup.py", line 431, in <module> **extra_opts File "/Users/pmital/.pyenv/versions/3.7.5/envs/pymongo_cp37/lib/python3.7/site-packages/setuptools/__init__.py", line 145, in setup return distutils.core.setup(**attrs) File "/Users/pmital/.pyenv/versions/3.7.5/lib/python3.7/distutils/core.py", line 148, in setup dist.run_commands() File "/Users/pmital/.pyenv/versions/3.7.5/lib/python3.7/distutils/dist.py", line 966, in run_commands self.run_command(cmd) File "/Users/pmital/.pyenv/versions/3.7.5/lib/python3.7/distutils/dist.py", line 985, in run_command cmd_obj.run() File "setup.py", line 123, in run self.test_suite) File "/Users/pmital/.pyenv/versions/3.7.5/lib/python3.7/unittest/loader.py", line 154, in loadTestsFromName module = __import__(module_name) File "/Users/pmital/Developer/mongo-python-driver/test/test_unified_format.py", line 46, in <module> 'FindOneAndUpdate returnDocument invalid enum value' File "/Users/pmital/Developer/mongo-python-driver/test/unified_format.py", line 989, in generate_test_classes fpath, schema_version)) ValueError: test file '/Users/pmital/Developer/mongo-python-driver/test/unified-test-format/valid-fail/schemaVersion-unsupported.json' has unsupported schemaVersion '0.1.0.0'

Note that with this change I have had to comment out the code that generates the valid-fail tests since leaving them would break CI (as they fail when run).

The other issue to consider is that there is now no way to run the other tests in the valid-fail directory without removing the schemaVersion-unsupported.json file. Previously, we continued to create test classes even after encountering a failure. Since we are now eagerly surfacing these errors to the user, this is no longer possible.
I don't think it will be a problem in practice (actual spec tests will never cause such a failure, and I think we'd be OK seeing a build failure if they did), but I thought I should point it out nonetheless.

ShaneHarvey · 2020-12-16T00:11:37Z

test/unified_format.py

+                    (mixin_class, test_base_class_factory(scenario_def),),
+                    {'__module__': module})
+            except (AttributeError, KeyError, TypeError):
+                print("Ignoring invalid test file '%s'\n"


Shouldn't we raise some kind of error here so that we actually notice when this happens?

Same answer as above.

ShaneHarvey · 2020-12-17T19:28:51Z

test/test_unified_format.py

+# globals().update(generate_test_classes(
+#     os.path.join(_TEST_PATH, 'valid-fail'),
+#     module=__name__,
+#     class_name_prefix='UnifiedTestFormat'))


Perhaps we can uncomment this by adding a flag expect_failure=True. When expect_failure=True we can ignore errors that happen at test construction time, if the test actually does get constructed then we should assert that test fails at runtime. What do you think?

That is too similar to expected_failures. How about a bypass_test_generation_errors kwarg instead that allows us to ignore/bypass errors raised during test creation?

test/unified_format.py

prashantmital · 2020-12-18T18:50:00Z

The remaining failures seem to be unrelated.

ShaneHarvey

Disable test commands variant is failing:

 [2020/12/21 20:08:32.700] ERROR [0.230s]: test_Aggregate_succeeds_after_InterruptedAtShutdown (test_unified_format.TestUnifiedTestFormatValidPassPocRetryableReads)
 [2020/12/21 20:08:32.700] ----------------------------------------------------------------------
 [2020/12/21 20:08:32.700] Traceback (most recent call last):
 [2020/12/21 20:08:32.700]   File "/data/mci/7bb1f7d1f5cfc16d0e9aeb8306ed5b7a/src/test/unified_format.py", line 923, in test_case
 [2020/12/21 20:08:32.700]     self.run_scenario(spec)
 [2020/12/21 20:08:32.700]   File "/data/mci/7bb1f7d1f5cfc16d0e9aeb8306ed5b7a/src/test/unified_format.py", line 907, in run_scenario
 [2020/12/21 20:08:32.700]     self.run_operations(spec['operations'])
 [2020/12/21 20:08:32.700]   File "/data/mci/7bb1f7d1f5cfc16d0e9aeb8306ed5b7a/src/test/unified_format.py", line 848, in run_operations
 [2020/12/21 20:08:32.700]     self.run_special_operation(op)
 [2020/12/21 20:08:32.700]   File "/data/mci/7bb1f7d1f5cfc16d0e9aeb8306ed5b7a/src/test/unified_format.py", line 840, in run_special_operation
 [2020/12/21 20:08:32.700]     method(spec['arguments'])
 [2020/12/21 20:08:32.700]   File "/data/mci/7bb1f7d1f5cfc16d0e9aeb8306ed5b7a/src/test/unified_format.py", line 756, in _testOperation_failPoint
 [2020/12/21 20:08:32.700]     command_args=spec['failPoint'])
 [2020/12/21 20:08:32.700]   File "/data/mci/7bb1f7d1f5cfc16d0e9aeb8306ed5b7a/src/test/unified_format.py", line 748, in __set_fail_point
 [2020/12/21 20:08:32.700]     client.admin.command(cmd_on)
 [2020/12/21 20:08:32.700]   File "/data/mci/7bb1f7d1f5cfc16d0e9aeb8306ed5b7a/src/pymongo/database.py", line 743, in command
 [2020/12/21 20:08:32.700]     codec_options, session=session, **kwargs)
 [2020/12/21 20:08:32.700]   File "/data/mci/7bb1f7d1f5cfc16d0e9aeb8306ed5b7a/src/pymongo/database.py", line 640, in _command
 [2020/12/21 20:08:32.700]     client=self.__client)
 [2020/12/21 20:08:32.700]   File "/data/mci/7bb1f7d1f5cfc16d0e9aeb8306ed5b7a/src/pymongo/pool.py", line 719, in command
 [2020/12/21 20:08:32.700]     exhaust_allowed=exhaust_allowed)
 [2020/12/21 20:08:32.700]   File "/data/mci/7bb1f7d1f5cfc16d0e9aeb8306ed5b7a/src/pymongo/network.py", line 161, in command
 [2020/12/21 20:08:32.700]     parse_write_concern_error=parse_write_concern_error)
 [2020/12/21 20:08:32.700]   File "/data/mci/7bb1f7d1f5cfc16d0e9aeb8306ed5b7a/src/pymongo/helpers.py", line 164, in _check_command_response
 [2020/12/21 20:08:32.700]     raise OperationFailure(errmsg, code, response, max_wire_version)
 [2020/12/21 20:08:32.700] OperationFailure: no such command: 'configureFailPoint', full error: {u'codeName': u'CommandNotFound', u'code': 59, u'ok': 0.0, u'errmsg': u"no such command: 'configureFailPoint'"}

https://evergreen.mongodb.com/task/mongo_python_driver_test_disableTestCommands__platform~rhel62_python_version~2.7_disableTestCommands~disabled_test_latest_standalone_patch_c673d8b3cea48f65615cf632fb287e1b9e57be72_5fdc2dc7d1fe07122697da8b_20_12_18_04_19_20

Probably missing a @require_test_commands for tests that use failPoints.

ShaneHarvey · 2020-12-21T21:56:53Z

Some more test failures (MMAPv1):

ShaneHarvey · 2020-12-22T00:40:19Z

test/unified_format.py

@@ -544,6 +544,19 @@ def setUpClass(cls):
            raise unittest.SkipTest(
                '%s runOnRequirements not satisfied' % (cls.__name__,))

+        # add any special-casing for skipping tests here
+        if client_context.storage_engine == 'mmapv1':
+            if cls.TEST_SPEC['description'].find('retryable-writes') != -1:


Nit: 'retryable-writes' in cls.TEST_SPEC['description'] instead of find() != -1

ShaneHarvey · 2020-12-22T00:42:05Z

test/unified_format.py

+            if cls.TEST_SPEC['description'].find(
+                    'transactions-convenient-api') != -1:
+                raise unittest.SkipTest(
+                    "MMAPv1 does not support document-level locking")


For these would it make more sense to skip any test that uses startTransaction()/withTransaction()/watch() on mmap rather than using strings (which can change)?

…pecify module in which dynamically created classes live

ShaneHarvey

LGTM

(cherry picked from commit 6b01235)

prashantmital requested a review from ShaneHarvey November 17, 2020 20:45

prashantmital commented Nov 17, 2020

View reviewed changes

test/test_unified_format.py Outdated Show resolved Hide resolved

ShaneHarvey reviewed Nov 17, 2020

View reviewed changes

test/unified_format.py Show resolved Hide resolved

test/test_unified_format.py Outdated Show resolved Hide resolved

prashantmital force-pushed the PYTHON-2033/unified-test-format branch from 6fc38e3 to d694867 Compare December 1, 2020 08:58

ShaneHarvey reviewed Dec 3, 2020

View reviewed changes

test/test_unified_format.py Outdated Show resolved Hide resolved

prashantmital marked this pull request as ready for review December 7, 2020 20:55

ShaneHarvey changed the title ~~Unified Test Format~~ PYTHON-2033 Unified Test Format Dec 8, 2020

ShaneHarvey reviewed Dec 8, 2020

View reviewed changes

prashantmital requested a review from ShaneHarvey December 10, 2020 21:49

prashantmital force-pushed the PYTHON-2033/unified-test-format branch from c6ad26c to 18e4056 Compare December 15, 2020 03:10

ShaneHarvey reviewed Dec 15, 2020

View reviewed changes

ShaneHarvey reviewed Dec 16, 2020

View reviewed changes

ShaneHarvey reviewed Dec 17, 2020

View reviewed changes

prashantmital force-pushed the PYTHON-2033/unified-test-format branch from 1207c2c to 735badf Compare December 17, 2020 22:59

prashantmital requested a review from ShaneHarvey December 17, 2020 23:14

ShaneHarvey requested changes Dec 18, 2020

View reviewed changes

test/unified_format.py Outdated Show resolved Hide resolved

ShaneHarvey requested changes Dec 21, 2020

View reviewed changes

ShaneHarvey reviewed Dec 22, 2020

View reviewed changes

prashantmital added 8 commits December 21, 2020 18:49

working changes

47998d8

skeletal solution

8a188cb

preserve case in test descriptions; remove unused imports; properly s…

5ed205a

…pecify module in which dynamically created classes live

tweak class name prefix specification

6ed7283

working commit 2

580645f

working commit 2

74a1cfe

working commit 3

9d51830

fixup change streams 1

fc21f1c

prashantmital added 25 commits December 21, 2020 18:49

get crud tests working

bef3a1a

further fleshing out

66d0148

get transactions tests working

5d8eeb1

remove cruft

a431875

add valid-fail testing

2beb719

add more operators

da8015f

implement more ops

9fbcaad

get \$type working

a2a9ee8

add support for sharded-replicaset topology

fdd7b69

remove cruft

e8aabd5

ignore sensitive cmds in monitoring

48e264f

normalize cmd names for comparison

0878405

ensure coll created

58cae3b

code review 1

d082993

improvements

6c49c96

more review

fd89761

more review 2

ff4cb56

fix handling of hint in bulk

b643b2d

fix

e66abf5

check test cmds enabled

aefb58d

skip mmap

4a3e066

fix 2

6a6681e

skip more tests on mmap

fb05c59

decentralize skipping code

5e00979

dont use find

855c752

prashantmital force-pushed the PYTHON-2033/unified-test-format branch from 7182e75 to 855c752 Compare December 22, 2020 02:50

ShaneHarvey approved these changes Dec 22, 2020

View reviewed changes

prashantmital merged commit 6b01235 into mongodb:master Dec 22, 2020

prashantmital deleted the PYTHON-2033/unified-test-format branch December 22, 2020 03:22

prashantmital added a commit that referenced this pull request Dec 22, 2020

PYTHON-2033 Unified Test Format (#519)

3a3aaee

(cherry picked from commit 6b01235)

PYTHON-2033 Unified Test Format #519

PYTHON-2033 Unified Test Format #519

Uh oh!

Conversation

prashantmital commented Nov 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ShaneHarvey left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

prashantmital commented Dec 10, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ShaneHarvey Dec 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

prashantmital commented Dec 16, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

prashantmital commented Dec 18, 2020

Uh oh!

ShaneHarvey left a comment

Choose a reason for hiding this comment

Uh oh!

ShaneHarvey commented Dec 21, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

prashantmital commented Nov 17, 2020 •

edited

Loading

ShaneHarvey Dec 15, 2020 •

edited

Loading