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

rozza · 2025-04-02T08:33:11Z

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

        testDocument.remove("resourcePath");
        testDocument.remove("fileName");
        return testDocument;
    }

-    public static Collection<Object[]> getTestData(final String resourcePath) {
+    public static Collection<Object[]> getLegacyTestData(final String resourcePath) {


Renamed to clarify the types of tests being loaded.

I don't think this is accurate, since it appears to be used not only by test runners using a format that will eventually be replaced by the unified one, but also once that are not intended to be replaced, e.g. connection string unit tests.

Will revert.

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.

jyemin · 2025-04-02T11:50:04Z

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

@@ -51,6 +51,9 @@ public class ServerDiscoveryAndMonitoringTest extends AbstractServerDiscoveryAnd
    public ServerDiscoveryAndMonitoringTest(final String description, final BsonDocument definition) {
        super(definition);
        this.description = description;
+        // Unified and monitoring tests have their own test runners


Is there a way to do this without skips? I'm concerned that it will be confusing, as it overloads the meaning of skipping a test, which is typically done because the driver has not implemented some feature or is otherwise incompatible with the test.

Filtered the data instead.

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.

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

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 added 3 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

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

jyemin Apr 2, 2025

rozza Apr 2, 2025

rozza Apr 2, 2025

rozza Apr 2, 2025

rozza Apr 2, 2025

rozza Apr 2, 2025

jyemin 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

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

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

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 •

edited

Loading

rozza commented Apr 2, 2025 •

edited

Loading