MONGOSH-400 - Use mlaunch for spawning test mongod processes #401
Conversation
addaleax
force-pushed
the
mlaunch
branch
2 times, most recently
from
5926734 to
d6a4b7a
Compare
| ['--single', '--replSet', replId], | ||
| ['--single', '--replSet', replId] | ||
| ); | ||
| let cfg: {_id: string, members: {_id: number, host: string, priority: number}[]}; |
There was a problem hiding this comment.
This is so cool I haven't seen this syntax before!
| internalState = new ShellInternalState(serviceProvider); | ||
| mongo = internalState.currentDb.getMongo(); | ||
| rs = new ReplicaSet(mongo); | ||
|
|
||
| // check replset uninitialized | ||
| try { | ||
| await rs.status(); | ||
| expect.fail(); |
There was a problem hiding this comment.
So mlaunch supports starting up uninitialized replsets? If it's not possible then we can potentially just skip the init test for now
There was a problem hiding this comment.
@aherlihy Yes, the tests here run & pass :) It doesn’t have a designated feature for starting uninitialized replsets, but you can pass pretty much any mongod server option through over it
addaleax
force-pushed
the
mlaunch
branch
11 times, most recently
from
6b56dea to
4e76028
Compare
|
Ah and there's one failing test: |
addaleax
force-pushed
the
mlaunch
branch
2 times, most recently
from
cededad to
b08f3c8
Compare
|
Ready for review, assuming that I didn’t manage to regress into breaking CI :) |
Switch to using `mlaunch` for spwaning mongod processes and enable the replica set and sharding tests. If merged in its current form, this would resolve - MONGOSH-400 (Decide how to spawn mongod processes) - MONGOSH-411 (Switch mongosh tests to use mlaunch) - MONGOSH-420 (Enable sharding tests) - MONGOSH-421 (Enable replica set tests)
| return new URL(this.connectionString()).hostname; | ||
| } | ||
|
|
||
| port(): string { |
There was a problem hiding this comment.
Maybe we should update this to be something other than 27017?
There was a problem hiding this comment.
I'm not sure I understand -- the default port for mongodb:// URLs is 27017, so we should return that if there's no port in the URL, right?
There was a problem hiding this comment.
Yes - that's right. I was thinking maybe we'd want to prevent tests from starting a server on 27017 (since most people have something else running on 27017 so if you start your test processes it may cause collisions), but it looks like none of the tests are doing that anyway and it would be incorrect/unexpected to always avoid 27017. So ignore my comment!
There was a problem hiding this comment.
The general idea here is that you can still specify MONGOSH_TEST_SERVER_URL for debugging if that helps to point to a specific server, and that in that case, if that URL does not have a port, we should use 27017 :)
Hm yeah ... it's been helpful for debugging this PR itself, but I can remove them/comment them out until we run into problems with these 👍 |
This is meant to finish up MONGOSH-400, i.e. investigating whether we can use mlaunch, rather than providing an actual working implementation, although this already works quite well imo. Evergreen Windows hosts apparently (?) do not have Python 3.x installed, that’s something we might need to solve before this could theoretically be merged. All in all, I’m pretty happy with how mlaunch itself works and the capabilities we get from it, in particular, it does enable running the replica set/sharding tests 🙂
Switch to using
mlaunchfor spwaning mongod processesand enable the replica set and sharding tests.
If merged in its current form, this would resolve