-
Notifications
You must be signed in to change notification settings - Fork 266
PHPLIB-530: Test against MongoDB 4.3 #723
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
Changes from all commits
3673d32
a1af358
4b5a0a1
c9b4f24
ea75303
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,7 @@ | |
namespace MongoDB\Model; | ||
|
||
use IteratorIterator; | ||
use Traversable; | ||
|
||
/** | ||
* CollectionInfoIterator for listCollections command results. | ||
|
@@ -32,6 +33,19 @@ | |
*/ | ||
class CollectionInfoCommandIterator extends IteratorIterator implements CollectionInfoIterator | ||
{ | ||
/** @var string|null */ | ||
private $databaseName; | ||
|
||
/** | ||
* @param string|null $databaseName | ||
*/ | ||
public function __construct(Traversable $iterator, $databaseName = null) | ||
{ | ||
parent::__construct($iterator); | ||
|
||
$this->databaseName = $databaseName; | ||
} | ||
|
||
/** | ||
* Return the current element as a CollectionInfo instance. | ||
* | ||
|
@@ -41,6 +55,12 @@ class CollectionInfoCommandIterator extends IteratorIterator implements Collecti | |
*/ | ||
public function current() | ||
{ | ||
return new CollectionInfo(parent::current()); | ||
$info = parent::current(); | ||
|
||
if ($this->databaseName !== null && isset($info['idIndex']) && ! isset($info['idIndex']['ns'])) { | ||
$info['idIndex']['ns'] = $this->databaseName . '.' . $info['name']; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this related to PHPLIB-499 (#718)? If so, I'm curious if the spec change needs to be revised. mongodb/specifications@8ed5d56 for SPEC-1399 only addressed the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, which is why I missed this in the original PR. |
||
} | ||
|
||
return new CollectionInfo($info); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,6 +14,13 @@ | |
*/ | ||
class CrudSpecTest extends FunctionalTestCase | ||
{ | ||
/** @var array */ | ||
private static $incompleteTests = [ | ||
'find-allowdiskuse: Find does not send allowDiskuse when value is not specified' => 'PHPLIB-500', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Noted that this depends on #721, which in turn is waiting for this PR to get merged. |
||
'find-allowdiskuse: Find sends allowDiskuse false when false is specified' => 'PHPLIB-500', | ||
'find-allowdiskuse: Find sends allowDiskUse true when true is specified' => 'PHPLIB-500', | ||
]; | ||
|
||
/** | ||
* Assert that the expected and actual command documents match. | ||
* | ||
|
@@ -37,6 +44,10 @@ public static function assertCommandMatches(stdClass $expected, stdClass $actual | |
*/ | ||
public function testCrud(stdClass $test, array $runOn = null, array $data, $databaseName = null, $collectionName = null) | ||
{ | ||
if (isset(self::$incompleteTests[$this->dataDescription()])) { | ||
$this->markTestIncomplete(self::$incompleteTests[$this->dataDescription()]); | ||
} | ||
|
||
if (isset($runOn)) { | ||
$this->checkServerRequirements($runOn); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doc block is inconsistent with the params.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the first argument is typed, it does not need a doc block to further clarify the type of the argument. I can add the
@param
annotation, but it doesn't add any value to the method.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I didn't realize this was just intended to document the union type. SGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep: we can't use nullable types yet as they were only introduced in PHP 7.1.