PHPLIB-500: Support for allowDiskUse on find operations #721
Conversation
| @@ -276,6 +276,10 @@ private function assertEquals(ArrayObject $expected, ArrayObject $actual, $ignor | |||
| } | |||
|
|
|||
| foreach ($expected as $key => $expectedValue) { | |||
| if ($expectedValue === null) { | |||
| continue; | |||
There was a problem hiding this comment.
I assume this was done to satisfy the Fields that must NOT be present in Actual Documents spec test change introduced by mongodb/specifications#692.
I think this is technically incorrect, as it's skipping the assertion entirely if the expected value is null. At the very least, I think you'd want to assert that $actualHasKey (set below) is false.\But rather than modify DocumentsMatchConstraint at all (which inadvertently affect other assertions, such as change stream results), I'd propose handling this in CrudSpecTest::assertCommandMatches(), similar to as is done in TransactionSpecTest here. I believe the transaction spec tests are the only other tests that use the same "null means unset" concept for command assertions.
Lastly, I believe this PR is also missing find-allowdiskuse.json from mongodb/specifications@92255e0.
There was a problem hiding this comment.
Good point. I've moved the check to not affect other tests. The find-allowdiskuse.json test was added in #717, but didn't get executed because we're not testing against a 4.3 server. Since some tests currently fail on 4.3, I'll add this in a separate PR and hold off on merging this until we're properly testing this functionality.
There was a problem hiding this comment.
Ah, I totally forgot that was included in my PR.
Good point. I've moved the check to not affect other tests.
Does this mean the change to DocumentsMatchConstraint.php above was reverted? If so, it wasn't pushed when you made this comment. I'll circle back later to review the change.
There was a problem hiding this comment.
Yep, I forgot to push this change 😅
|
@jmikola holding off on merging this until PHPLIB-530 is done as the functionality introduced here currently isn't tested for lack of a 4.3 build stage. |
https://jira.mongodb.org/browse/PHPLIB-500
PHP 5 build failures will be fixed by #720.