PHPC-1173: Reimplement replica set seedlist tests #906
Conversation
tests/utils/skipif.php
Outdated
| @@ -39,6 +39,20 @@ function skip_if_not_replica_set() | |||
| is_replica_set(URI) or exit('skip topology is not a replica set'); | |||
| } | |||
|
|
|||
| /** | |||
| * Skips the test if the topology is not a replica set. | |||
There was a problem hiding this comment.
This comment should be updated to refer to an arbiter.
tests/utils/skipif.php
Outdated
| function skip_if_no_arbiter() | ||
| { | ||
| is_replica_set(URI) or exit('skip topology is not a replica set'); | ||
| $primary = get_primary_server(URI); |
There was a problem hiding this comment.
I think this should follow the model in skip_if_not_live() with a try/catch for ConnectionException. That way, it's not relying on our global exception handler to emit a "skip" message.
There was a problem hiding this comment.
The is_replica_set call shouldn't have been here. And the test(s) that use skip_if_no_arbiter already check the skip_if_not_replica_set first too.
As a matter of fact, is_replica_set/skip_if_replica_set don't do the try/catch either…
So although I can wrap the 48 line in a try-catch, I am not sure what it actually improves.
| --FILE-- | ||
| <?php | ||
| require_once __DIR__ . "/../utils/basic.inc"; | ||
|
|
||
| $secondary = get_secondary_server(URI); |
There was a problem hiding this comment.
I think this calls for a skip_if_no_secondary() function in the SKIPIF, as this could easily fail against a replica set with a single primary node.
There was a problem hiding this comment.
Added, but we don't have such an (automated) set-up in Travis. Should we add one?
There was a problem hiding this comment.
I'm fine without if you'd like to test this locally. It's something we should certainly intend to test on Evergreen, but we should be fairly conservative in our use of Travis' resources :)
There was a problem hiding this comment.
I added one anyway...
|
|
||
| $dsn = "mongodb://192.168.112.10:3001/?replicaSet=REPLICASET"; | ||
| $manager = new MongoDB\Driver\Manager($dsn); | ||
| $dsn = 'mongodb://' . $info['me']; |
derickr
force-pushed
the
PHPC-1173-replica-set-seedlist-tests
branch
2 times, most recently
from
c87b5c0 to
70f8f24
Compare
tests/utils/skipif.php
Outdated
| try { | ||
| $primary = get_primary_server(URI); | ||
| } catch (ConnectionException $e) { | ||
| exit('skip server is not accessible: ' . $e->getMessage()); |
There was a problem hiding this comment.
Should this message also say "primary server" for consistency?
There was a problem hiding this comment.
Will fix before merging.
derickr
force-pushed
the
PHPC-1173-replica-set-seedlist-tests
branch
from
70f8f24 to
7d1ac7b
Compare
No description provided.