Added mongodb/specifications as a git submodule #1670

rozza · 2025-04-01T13:30:13Z

Sets the specifications submodule to last schemaVersion 1.21 commit.
Deleted old copied json tests.
Updated test runners to use the new submodule locations
Added skips for any tests that haven't been implemented

Set the specifications submodule to last schemaVersion 1.21 commit Deleted old copied json tests. Updated test runners to use the new submodule locations Added skips for any tests that haven't been implemented JAVA-5821

bson/src/test/unit/util/JsonPoweredTestHelper.java

rozza · 2025-04-02T08:34:05Z

bson/src/test/unit/util/JsonPoweredTestHelper.java

@@ -68,10 +67,19 @@ public static Collection<Object[]> getTestData(final String resourcePath) {
        return data;
    }

+    public static List<BsonDocument> getSpecTestDocuments(final String resourcePath) {


New helper to auto include the specifications location prefix.

rozza · 2025-04-02T08:36:49Z

bson/src/test/unit/util/JsonPoweredTestHelper.java

        }
        return files;
    }

    private static BsonDocument getTestDocumentWithMetaData(final String resourcePath) {
-        JsonReader jsonReader = new JsonReader(resourcePathToString(resourcePath));
-        BsonDocument testDocument = new BsonDocumentCodec().decode(jsonReader, DecoderContext.builder().build());
+        BsonDocument testDocument = BsonDocument.parse(resourcePathToString(resourcePath));


Simplification of the parsing code.

rozza · 2025-04-02T09:01:11Z

driver-sync/src/test/functional/com/mongodb/client/ClientSideEncryption25LookupProseTests.java

@@ -72,6 +72,7 @@ protected ClientEncryption createClientEncryption(final ClientEncryptionSettings

    @BeforeEach
    public void setUp() {
+        assumeFalse(() -> true); // https://jira.mongodb.org/browse/JAVA-5837


This is required as at the schema version 1.21 commit of the specifications repo we doesn't contain have the test files yet

rozza · 2025-04-02T09:06:54Z

driver-sync/src/test/functional/com/mongodb/client/unified/UnifiedTestModifications.java

@@ -474,7 +531,7 @@ public TestApplicator testContains(final String dir, final String fragment) {
         * @param test the individual test's "description" field
         * @return this
         */
-        public TestApplicator test(final String dir, final String test) {
+        public TestApplicator debug(final String dir, final String test) {


@katcharov I renamed this method as I found it too easy to use thinking it was a matcher.

driver-core/src/test/unit/com/mongodb/internal/connection/ServerDiscoveryAndMonitoringTest.java

jyemin

I looked at some of the tasks on the patch build and found a handful of tests that are passing on the mainline and skipped in the patch. Easiest way I found to locate them is to filter by Status, including only Skip, then sort Base Status no way to filter by Base Status unfortunately) and scroll to ones with Base Status of Passed. As examples, I found these in test-core:

com.mongodb.AuthConnectionStringTest.shouldPassAllOutcomes_should_accept_generic_mechanism_property__GSSAPI__
com.mongodb.UriOptionsTest.shouldPassAllOutcomes_Valid_auth_options_are_parsed_correctly__GSSAPI__
com.mongodb.UriOptionsTest.shouldPassAllOutcomes_tlsAllowInvalidCertificates_is_parsed_correctly_
com.mongodb.internal.connection.ConnectionPoolAsyncTest.shouldPassAllOutcomes_pool-create-min-size-error.json__error_during_minPoolSize_population_clears_pool_
com.mongodb.internal.connection.ConnectionPoolTest.shouldPassAllOutcomes_pool-create-min-size-error.json__error_during_minPoolSize_population_clears_pool_

and these in kotlkin-tests:

com.mongodb.kotlin.client.coroutine.UnifiedCrudTest.find__Find_with_batchSize_equal_to_limit
com.mongodb.kotlin.client.coroutine.UnifiedCrudTest.bulkWrite-comment__BulkWrite_with_string_comment
com.mongodb.kotlin.client.coroutine.UnifiedCrudTest.bulkWrite-comment__BulkWrite_with_document_comment
com.mongodb.kotlin.client.UnifiedCrudTest.find__Find_with_batchSize_equal_to_limit
com.mongodb.kotlin.client.UnifiedCrudTest.bulkWrite-comment__BulkWrite_with_string_comment
com.mongodb.kotlin.client.UnifiedCrudTest.bulkWrite-comment__BulkWrite_with_document_comment

Not sure of the root cause, but seems worth investigating.

Also, responding to:

Sets the specifications submodule to last schemaVersion 1.21 commit

I assume this will have to be updated to the latest commit before this PR is merged, otherwise we will be missing tests updated made in the last few months?

Update with one other thought: now that we are beholden to the directory structure of the specifications repository, there is a greater risk that a restructuring of that repository could cause us to miss an entire test suite (say, if the directory moves). Is there already a check to ensure that every test runner finds at least one test to consider running, and if not, could one be added?

rozza · 2025-04-02T17:14:00Z

@jyemin thanks for the review:

com.mongodb.AuthConnectionStringTest.shouldPassAllOutcomes_should_accept_generic_mechanism_property__GSSAPI__
skipped due to JAVA-4278. We don't support forward yet. This was different on main.
com.mongodb.UriOptionsTest.shouldPassAllOutcomes_Valid_auth_options_are_parsed_correctly__GSSAPI__
As previous.
com.mongodb.UriOptionsTest.shouldPassAllOutcomes_tlsAllowInvalidCertificates_is_parsed_correctly_
will fix I took the // Skip because Java driver does not support the tlsAllowInvalidCertificates option code comment in UriOptionsTests literally.
com.mongodb.internal.connection.ConnectionPoolAsyncTest.shouldPassAllOutcomes_pool-create-min-size-error.json__error_during_minPoolSize_population_clears_pool_
events in specifications are in a different order. I had added a comment to code: Events out of order - the driver closes connection first then clears the pool
com.mongodb.internal.connection.ConnectionPoolTest.shouldPassAllOutcomes_pool-create-min-size-error.json__error_during_minPoolSize_population_clears_pool_
As previous.

The kotlin tests:

com.mongodb.kotlin.client.coroutine.UnifiedCrudTest.find__Find_with_batchSize_equal_to_limit
Was skipped for all drivers - I will revert I no longer recall why.
com.mongodb.kotlin.client.coroutine.UnifiedCrudTest.bulkWrite-comment__BulkWrite_with_string_comment
Driver does not batch replace and updates together - was modified in main.
com.mongodb.kotlin.client.coroutine.UnifiedCrudTest.bulkWrite-comment__BulkWrite_with_document_comment
As previous.
com.mongodb.kotlin.client.UnifiedCrudTest.find__Find_with_batchSize_equal_to_limit
See coroutine
com.mongodb.kotlin.client.UnifiedCrudTest.bulkWrite-comment__BulkWrite_with_string_comment
See coroutine
com.mongodb.kotlin.client.UnifiedCrudTest.bulkWrite-comment__BulkWrite_with_document_comment
See coroutine

rozza · 2025-04-02T17:36:49Z

Also, responding to:

Sets the specifications submodule to last schemaVersion 1.21 commit

I assume this will have to be updated to the latest commit before this PR is merged, otherwise we will be missing tests updated made in the last few months?

No the latest commit contains work we haven't done yet but we can go to the last 1.22 commit which will include tests we have added and I've temporarily skipped. Those are all in progress / blocked by this PR. There is a Java ticker to support 1.23 - JAVA-5792

Update with one other thought: now that we are beholden to the directory structure of the specifications repository, there is a greater risk that a restructuring of that repository could cause us to miss an entire test suite (say, if the directory moves). Is there already a check to ensure that every test runner finds at least one test to consider running, and if not, could one be added?

We definitely could miss new tests - but then there should be a drivers ticket along the change to ensure its included. That would be the manual verification step.

For unified tests - we could remove all runners and just include a single runner. That traverses all directories in source/ and gathers all unified spec tests. That would ensure no unified tests were missed.

I will add an assertion to ensure getTestData / getTestDocuments does not return an empty list.

jyemin · 2025-04-02T18:02:19Z

All sounds good. Please though take a quick look at other tasks, as there are more in driver-sync that are also skipped in this patch but passing in main. I didn't enumerate them all. Probably good reason for all of them but this is the time to check carefully.

rozza · 2025-04-03T08:58:52Z

@jyemin all skips are expected - either we modified the tests / tests are different in specifications. Or there is a reason for the skip - eg timeoutMS too small.

More tests will be reintroduced in subsequent prs as the schema support is updated.

jyemin

Looks great, just a couple of questions left.

jyemin · 2025-04-22T17:34:03Z

driver-sync/src/test/functional/com/mongodb/client/ClientSideOperationTimeoutTest.java

+         * The test is occasionally racy. The "killCursors" command may appear as an additional event. This is unexpected in unified tests,
+         * but anticipated in reactive streams because an operation timeout error triggers the closure of the stream/publisher.
+         */
+        ignoreExtraCommandEvents(testDescription.contains("timeoutMS is refreshed for getMore - failure"));


Looking at the comment, I'm not clear why this is being added to the sync test rather than the reactive one. Can you clarify?

Thank you - that was copy pasta and wasn't the actual test I should have been ignoring extra events for. Have updated. Test failures can be seen here - Search for: com.mongodb.client.ClientSideOperationTimeoutTest.timeoutMS_behaves_correctly_for_tailable_awaitData_cursors__timeoutMS_is_refreshed_for_getMore_if_maxAwaitTimeMS_is_set

jyemin · 2025-04-22T17:39:18Z

driver-core/src/test/unit/com/mongodb/internal/connection/AbstractConnectionPoolTest.java

+        assumeFalse(fileName.equals("pool-clear-interrupting-pending-connections.json"));
+
+        // Events out of order - the driver closes connection first then clears the pool.
+        assumeFalse(fileName.equals("pool-create-min-size-error.json"));


Should we track this with a ticket?

I think that would be a drivers ticket. I'll look to see if there is a ticket already.

Added the related Java ticket

rozza · 2025-04-23T12:08:29Z

@jyemin I think all other test failures are now not related to specifications.

#1684 fixes the majority of the change stream tests due to changes with the latest mongodb.

jyemin · 2025-04-23T12:22:51Z

Looking good! I had a glance at the latest test run and was wondering about ClientEncryptionCustomEndpointTest.testCustomEndpoint_4.__aws__invalid_amazon_region_in_endpoint_, which is failng on both sync and reactice tasks, but passing on mainline. Do you know what's going on with those tests?

rozza · 2025-04-23T13:44:44Z

@jyemin those tests appear to also be failing in main:
https://spruce.mongodb.com/version/mongo_java_driver_b7cb24a586ddd164de9672ee1741ce3f3f19f732/test-analysis?sorts=STATUS%3AASC%3BBASE_STATUS%3ADESC

Bailey commented yesterday in dbx-devs - so possibly seen elsewhere as well.

jyemin

LGTM

rozza marked this pull request as draft April 1, 2025 16:06

rozza force-pushed the JAVA-5821 branch from 95b5433 to 0190996 Compare April 1, 2025 18:00

rozza marked this pull request as ready for review April 2, 2025 08:31

rozza requested review from a team, stIncMale and katcharov and removed request for a team April 2, 2025 08:32

rozza force-pushed the JAVA-5821 branch from 0190996 to b966e7f Compare April 2, 2025 08:56

Added mongodb/specifications as a git submodule

dcccfd6

Set the specifications submodule to last schemaVersion 1.21 commit Deleted old copied json tests. Updated test runners to use the new submodule locations Added skips for any tests that haven't been implemented JAVA-5821

rozza force-pushed the JAVA-5821 branch from b966e7f to dcccfd6 Compare April 2, 2025 09:03

rozza commented Apr 2, 2025

View reviewed changes

Deleted findOne.json - left over from merge

afa3c5d

jyemin reviewed Apr 2, 2025

View reviewed changes

driver-core/src/test/unit/com/mongodb/internal/connection/ServerDiscoveryAndMonitoringTest.java Outdated Show resolved Hide resolved

jyemin self-requested a review April 2, 2025 15:01

jyemin requested changes Apr 2, 2025

View reviewed changes

katcharov removed the request for review from stIncMale April 2, 2025 16:05

rozza added 3 commits April 2, 2025 18:14

Code review updates

2d11d89

Filter the data rather than skip

f8a2fe5

Revert getLegacyTestData name change

179e3b4

rozza added 2 commits April 2, 2025 18:39

Ensure finding no files throws an assertion error

5f613c4

Improve assertion error message

2302f5f

rozza requested a review from jyemin April 2, 2025 17:43

Checkstyle fixes and assertion improvements

e12b76c

rozza added 3 commits April 3, 2025 10:00

Merge branch 'main' into JAVA-5821

4b0af96

Update specifications submodule to main

21cb0b5

Update test runner and revert lookup test skipping

7a139b1

rozza and others added 5 commits April 3, 2025 18:35

Skip test until JAVA-5297

ce9a44c

Skip test until JAVA-5815

a72441e

Ensure schema version is greater that 1

9977755

Merge branch 'main' into JAVA-5821

a4f3bea

Getmores in sync can also be racy for tailable cursors

cc74980

jyemin reviewed Apr 22, 2025

View reviewed changes

rozza added 4 commits April 23, 2025 09:17

Ignore extra getMores in sync test with timeoutMS

53dedba

Add Java ticket to assume false

ac83a37

Skip nsType is present when creating views JAVA-5769

8d1b93e

Correct file description

21f8f1a

rozza requested a review from jyemin April 23, 2025 12:05

jyemin approved these changes Apr 23, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added mongodb/specifications as a git submodule #1670

Added mongodb/specifications as a git submodule #1670

rozza commented Apr 1, 2025

rozza Apr 2, 2025

rozza Apr 2, 2025

rozza Apr 2, 2025

rozza Apr 2, 2025

jyemin left a comment •

edited

Loading

rozza commented Apr 2, 2025

rozza commented Apr 2, 2025 •

edited

Loading

jyemin commented Apr 2, 2025

rozza commented Apr 3, 2025

jyemin left a comment

jyemin Apr 22, 2025

rozza Apr 23, 2025 •

edited

Loading

jyemin Apr 22, 2025

rozza Apr 23, 2025

rozza Apr 23, 2025

rozza commented Apr 23, 2025

jyemin commented Apr 23, 2025

rozza commented Apr 23, 2025

jyemin left a comment

Added mongodb/specifications as a git submodule #1670

Are you sure you want to change the base?

Added mongodb/specifications as a git submodule #1670

Conversation

rozza commented Apr 1, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jyemin left a comment • edited Loading

Choose a reason for hiding this comment

rozza commented Apr 2, 2025

rozza commented Apr 2, 2025 • edited Loading

jyemin commented Apr 2, 2025

rozza commented Apr 3, 2025

jyemin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rozza Apr 23, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rozza commented Apr 23, 2025

jyemin commented Apr 23, 2025

rozza commented Apr 23, 2025

jyemin left a comment

Choose a reason for hiding this comment

jyemin left a comment •

edited

Loading

rozza commented Apr 2, 2025 •

edited

Loading

rozza Apr 23, 2025 •

edited

Loading