tests: fix/optimise download-timeout spread test #13967
Conversation
The download-timeout test was taking over 10min to complete due to a couple of issues. The first one is that there was a typo in the environment variables that controlled the measurement window so the timeout was still the default of 5min (the test downloads twice). Setting it correctly cut the execution time for one of the downloads but not the other strangely. It turns out that the test was creating a override file that was sorted before the other existing override (systemd applies env vars in order). Fixing this and lowering the timeout to 15s cuts the running time from 10min to 30s. I wanted to add a log when the env var was set so we could check it in the test but this happens in an `init()` and I don't think the logger is set up by then. Signed-off-by: Miguel Pires <miguel.pires@canonical.com>
MiguelPires
requested review from
andrewphelpsj and
bboozzoo
and removed request for
andrewphelpsj
| Environment=SNAP_DOWNLOAD_MEAS_WINDOW=${SNAP_DOWNLOAD_MEAS_WINDOW} | ||
| EOF | ||
| cp "$OVERRIDES_FILE" "$OVERRIDES_FILE".bak | ||
| sed "s/Environment=/Environment=SNAPD_MIN_DOWNLOAD_SPEED=${SNAPD_MIN_DOWNLOAD_SPEED} SNAPD_DOWNLOAD_MEAS_WINDOW=${SNAPD_DOWNLOAD_MEAS_WINDOW} /" -i "$OVERRIDES_FILE" |
There was a problem hiding this comment.
Hmm this is changing an overrides file we created during prepare if I'm not mistaken. Wouldn't it be cleaner to add a separate file like the test did before, but make sure it's named such that it gets included after local.conf?
There was a problem hiding this comment.
It is but the file is restored to its original state so it should be ok. I considered renaming the file but my concern was that something else would change and break this silently, eg. we rename local.conf to something that gets sorted after the override like snapd.conf (or worse, znapd.conf 😛). IIRC all other tests I found changed local.conf instead of creating overrides. But I could change it to z-env-override.conf and leave a comment saying that this is necessary and why
There was a problem hiding this comment.
I see there are only 7 tests that seem to touch local.conf. How about we try to fix it for good then. Say we could go with something like 00-prepare.conf in prepare.sh, then each test could use a consistent 90-${SPREAD_TASK}.conf, this way the original file created during prepare does not get modified, the cleanup is easy and if something is not cleaned up we can tell which test left a file. (also writing an invariant check would be easy now, simply check 00-prepare.conf is the only file under /etc/systemd/system/snapd.service.d/. WDYT?
There was a problem hiding this comment.
This sounds like a great idea. Being able to check if any configuration files were left and knowing which test to blame seems very useful. I'll open a follow-up. thanks!
The download-timeout test was taking over 10min to complete due to a couple of issues. The first one is that there was a typo in the environment variables that controlled the measurement window so the timeout was still the default of 5min (the test downloads twice). Setting it correctly cut the execution time for one of the downloads but not the other strangely. It turns out that the test was creating a override file that was sorted before the other existing override (systemd applies sorts override files). Fixing this and lowering the timeout to 15s cuts the running time from 10min to 30s. I wanted to add a log when the env var was set so we could check it in the test but this happens in an
init()and I don't think the logger is set up by then.