refactor: enable parallel ci bats tests by enabling dfx start and bootstrap to pick random webserver ports#920
Conversation
p-shahi
changed the title
|
@basvandijk If I rebase this branch, it will get evaluation errors like so: https://hydra.dfinity.systems/jobset/dfinity-ci-build/sdk.pr-918#tabs-errors |
src/dfx/src/commands/start.rs
Outdated
| if args.is_present("background") { | ||
| send_background()?; | ||
| return ping_and_wait(&frontend_url); | ||
| return Ok(()); |
There was a problem hiding this comment.
We should still leave the process when the frontend is available, so ping_and_wait should stay.
There was a problem hiding this comment.
For the 0 port case, doing ping_and_wait will fail: If the user does dfx start --host 127.0.0.1:0 the foreground process will attempt to ping_and_wait("http://127.0.0.1:0") when the port it should actually use is only allocated to the background process.
Since we still call ping_and_wait in the bg process here https://github.com/dfinity-lab/sdk/blob/54e3b812e0351f5f3e55f8a9aeedcfabc7e7f3cb/src/dfx/src/commands/start.rs#L196 I though it was ok to delete it.
There was a problem hiding this comment.
I agree that we should wait until ping succeeds. Can we do that by waiting for the background process to write its port into webserver-port, and then using that to ping?
There was a problem hiding this comment.
Np, done. PTAL @ericswanson-dfinity
Running the e2e bats test sequentially took 15m on my laptop. We should run them all in parallel such that we don't have to wait so long on CI.
p-shahi
force-pushed
the
pshahi/parallel-tests-changes
branch
from
54e3b81 to
f303e36
Compare
src/dfx/src/commands/start.rs
Outdated
| if args.is_present("background") { | ||
| send_background()?; | ||
| return ping_and_wait(&frontend_url); | ||
| return Ok(()); |
There was a problem hiding this comment.
I agree that we should wait until ping succeeds. Can we do that by waiting for the background process to write its port into webserver-port, and then using that to ping?
| thiserror = "1.0.20" | ||
| toml = "0.5.5" | ||
| tokio = "0.2.10" | ||
| tokio = { version = "0.2.10", features = [ "fs" ] } |
There was a problem hiding this comment.
…ource for webserver_port to be written
|
How can we run e2e tests locally after this change? These are what I usually run: These are both documented in the contributing document. Could you please update it? |
Yep, will update docs. Both will work by doing Related question, do you ever do this workflow: |
|
No, I have never seen that workflow! Here is what happened when I tried it: |
3 participants
I'm going to merge these commits into #877
So far this PR does a couple things
dfx startdfx bootstrapanddfx startcould not handle the user providing port0i.e. telling dfx to pick a random port for the webserver.dfx/webserver-port(similar to.dfx/pid) so that it can be queried by dfx (or by the bats test)the second and third bullet will enable the bats tests to run in parallel in darwin (w/o network sandbox) since we'll tell dfx start or dfx bootstrap to use port
0, the kernel will dynamically allocate a port, and the bats test can read.dfx/webserver-port