Add legacy solver setup for e2e tests #2284
Conversation
devanoneth
force-pushed
the
feat/legacy-solver-e2e-setup
branch
from
9a19623 to
5dbe756
Compare
crates/e2e/src/setup/services.rs
Outdated
| let solver_endpoint = | ||
| solver_endpoint.unwrap_or("http://localhost:8000/solve".parse().unwrap()); | ||
| let solver_endpoint = start_legacy_solver(solver_endpoint).await; |
There was a problem hiding this comment.
nit: Can we make it slightly clearer that these two are quite different. One is the actual external legacy solver endpoint and one is the colocated legacy solver engine.
There was a problem hiding this comment.
Good point, thanks. Fixed in latest commit.
crates/e2e/src/setup/services.rs
Outdated
| @@ -91,7 +91,7 @@ impl<'a> Services<'a> { | |||
| "--auction-update-interval=1s".to_string(), | |||
| format!("--ethflow-contract={:?}", self.contracts.ethflow.address()), | |||
| "--skip-event-sync=true".to_string(), | |||
| "--solve-deadline=2s".to_string(), | |||
| "--solve-deadline=11s".to_string(), | |||
There was a problem hiding this comment.
I assume this isn't messing with our tests since baseline/naive return early and never use the full time? Otherwise we could also make it configurable.
There was a problem hiding this comment.
I had the same though and wanted to confirm by running CI once before commenting. Indeed it looks like this would be an issue for us. The CI time went from ~5m to ~10m.
The reason is that even if the baseline solver is able to compute a solution way faster the driver delays the submission of the solution until shortly before the deadline in case the solution would start to revert at some point.
There was a problem hiding this comment.
Ah right, I also figured it was a max time. But I checked back on the logic where you implemented this and, as you mentioned, I see the driver then delays until the end of the duration.
I would suggest to add a solve_deadline: Option<Duration> argument to start_autopilot, where it defaults to 2s if None. I feel like a separate function with only one difference is overkill but it would make the diff smaller. What do you think?
There was a problem hiding this comment.
In the case that you agree, I've committed this change now. Happy to change the approach if you want though.
|
Everything seems to be working except that GitHub CI doesn't read the |
Description
Since the collocated driver upgrades, at Enso we've been running our e2e tests against quite an old version of this repo. These small changes would help us upgrade our
servicesdependency to the latest commits.Changes
e2esetup module which help external solvers use the e2e test harness in their own codebases.solve-deadlineto 11s from 2s so that external solvers have a more realistic duration to return a solution in e2e tests.How to test