Skip to content

PHPLIB-461: Fix functional tests for change streams on sharded clusters #659

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Nov 25, 2019
Merged

PHPLIB-461: Fix functional tests for change streams on sharded clusters #659

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
32bade4
PHPLIB-461: Fix ChangeStream tests on sharded clusters
alcaeus Aug 2, 2019
944e729
PHPLIB-461: Lower periodicNoopIntervalSecs
alcaeus Aug 2, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 20 additions & 4 deletions mongo-orchestration/sharded_clusters/cluster_replset.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,11 @@
"journal": true,
"logappend": true,
"port": 4400,
"bind_ip_all": true
"bind_ip_all": true,
"setParameter": {
"periodicNoopIntervalSecs": 1,
"writePeriodicNoops": true
}
} },
{ "procParams": {
"dbpath": "/tmp/SHARDED-RS/SHARD1/4401",
Expand All @@ -59,7 +63,11 @@
"journal": true,
"logappend": true,
"port": 4401,
"bind_ip_all": true
"bind_ip_all": true,
"setParameter": {
"periodicNoopIntervalSecs": 1,
"writePeriodicNoops": true
}
} }
]
}
Expand All @@ -76,7 +84,11 @@
"journal": true,
"logappend": true,
"port": 4410,
"bind_ip_all": true
"bind_ip_all": true,
"setParameter": {
"periodicNoopIntervalSecs": 1,
"writePeriodicNoops": true
}
} },
{ "procParams": {
"dbpath": "/tmp/SHARDED-RS/SHARD2/4411",
Expand All @@ -85,7 +97,11 @@
"journal": true,
"logappend": true,
"port": 4411,
"bind_ip_all": true
"bind_ip_all": true,
"setParameter": {
"periodicNoopIntervalSecs": 1,
"writePeriodicNoops": true
}
} }
]
}
Expand Down
4 changes: 4 additions & 0 deletions tests/DocumentationExamplesTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -935,6 +935,10 @@ public function testChangeStreamExample_1_4()
{
$this->skipIfChangeStreamIsNotSupported();

if ($this->isShardedCluster()) {
$this->markTestSkipped('Test does not apply on sharded clusters: need more than a single getMore call on the change stream.');
}

$db = new Database($this->manager, $this->getDatabaseName());
$db->dropCollection('inventory');
$db->createCollection('inventory');
Expand Down
3 changes: 0 additions & 3 deletions tests/FunctionalTestCase.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,9 +336,6 @@ protected function skipIfChangeStreamIsNotSupported()
if (! $this->isShardedClusterUsingReplicasets()) {
$this->markTestSkipped('$changeStream is only supported with replicasets');
}

// Temporarily skip tests because of an issue with change streams in the driver
$this->markTestSkipped('$changeStreams currently don\'t on replica sets');
Copy link
Member

Choose a reason for hiding this comment

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

The PR itself LGTM, but I realized that removing this skip re-introduced the test failures.

I assume those were not happening in previous Travis builds since this skip was first introduced in 2008bc7 as part of #655?

We can discuss offline about whether to merge as-is for now and defer sorting out the double-free for replica set and sharded replica set tests in PHPLIB-432.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes and no. Not sure what made the errors appear consistently, but the skip only affected sharded clusters, not the replicaset build. With this branch, the replicaset build always fails while the build on a sharded cluster at least passes sometimes.

Since this entire PR deals with fixing tests on sharded clusters, I don't see a reason why to merge this without running the tests on a sharded cluster. I'd first like to investigate the cause for the double free some more before just plain skipping those tests that cause the failures.

break;

case Server::TYPE_RS_PRIMARY:
Expand Down
Loading