Skip to content
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.

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

PHPLIB-1404: Convert retryable reads spec tests to unified test format #1247

Merged
merged 5 commits into from
Mar 11, 2024

Conversation

jmikola
Copy link
Member

@jmikola jmikola commented Mar 6, 2024

}
},
"expectResult": {
"$$matchesHexBytes": "11"
Copy link
Member Author

Choose a reason for hiding this comment

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

Related to mongodb/specifications#1536

In findChunksByFileId(), we add additional criteria to the chunks query beyond the files_id expected in the original tests. The legacy test format permitted extra fields at all levels, so this was never checked.

I could have worked around this with $$matchAsRoot, but PHPLIB hasn't implemented that yet. Instead, I opted to relax the commandStartedEvent query to ignore filter altogether and instead assert the result of the successful operation (which was never done in the original tests).

* @see https://github.com/mongodb/specifications/tree/master/source/retryable-reads
* @group serverless
*/
class RetryableReadsSpecTest extends FunctionalTestCase
Copy link
Member Author

Choose a reason for hiding this comment

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

Note: PHP has yet to implement any of the retryable reads prose tests. The CMAP prose test doesn't apply, and the mongos-related tests are waiting on PHPC-1911.

As such, there was nothing else in this file that needed to be kept.


return $collection->mapReduce(
return iterator_to_array($collection->mapReduce(
Copy link
Member Author

Choose a reason for hiding this comment

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

Without this, the test runner attempted to match a MapReduceResult directly and timed out (likely due to some loop when dumping the comparison failure). This is consistent with eager iteration for other cursor-returning operations.

While looking into this, I wondered by we're still handling write result classes in ExpectedResult instead of immediately after an Operation. Opened PHPLIB-1406 to revisit that.

This works around an error for a distinct operation that references an entity created by a createEntities test runner operation.
* be created by a createEntities test runner operation, the
* assertion below will fail; however, there is no need to
* address this until such a transaction test is created. */
$collection = $context->getEntityMap()[$operation->object] ?? null;
Copy link
Member Author

Choose a reason for hiding this comment

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

Working around this would require running the createEntities operation ahead of time, which is a can of worms.

Copy link
Member

@alcaeus alcaeus left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for calling out the potential issue with entities created in createEntities. Fine leaving this as is for the time being.

@jmikola jmikola merged commit a303317 into mongodb:master Mar 11, 2024
14 checks passed
@jmikola jmikola deleted the phplib-1404 branch March 11, 2024 13:24
alcaeus added a commit that referenced this pull request May 10, 2024
* v1.18: (50 commits)
  Enable auto-merge in merge-up workflow (#1295)
  PHPLIB-1447: Add SBOM lite (#1292)
  Fix syntax error in docs (#1285)
  PHPLIB-1163 Create tutorial for using MongoDB with Bref (#1273) (#1282)
  Create sarif report when running psalm (#1280)
  Update composer.json and CI matrices for 1.18.0
  PHPLIB-1410: Invoke drivers-evergreen-tools scripts with bash (#1267)
  PHPLIB-1302: Use Composer\InstalledVersions (#1262)
  PHPLIB-1320: Support "comment" command option in Collection::createIndex() (#1263)
  PHPLIB-1413: Use env instead of matrix for driver-version (#1261)
  Fix Markdown heading
  PHPLIB-1399: Docs examples for agg expr projection (#1260)
  PHPLIB-1412: Skip range encryption tests on MongoDB 8.0+ (#1259)
  PHPLIB-1409: Skip $out and mapReduce tests on serverless (#1254)
  Exclude read-write-concern tests from serverless testing (#1253)
  PHPLIB-1409: Convert default write concern tests to unified test format (#1252)
  PHPLIB-1408: Convert ADL spec test to unified test format (#1250)
  Remove redundant annotations (#1251)
  PHPLIB-1404: Convert retryable reads spec tests to unified test format  (#1247)
  DOCSP-36627: Additional double backslash fixes for master (#1246)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants