Description
During a code review Peter Searby said
It feels like we should have caught the missing offline upgrade case with a test.
I remember last time we forgot, and it wasn't until I tested it manually that I saw that it failed.We currently have a test which tests that offline upgrade can be performed from min supported version to current version. The reason that test doesn't fail is that we get to this clause, and set the config_version to CurrentVersion after reaching the last version we support upgrade from, in this case it would have been 7.2. So we successfully upgrade to the current version, which in this case would have been 7.7, and the test passes.
However, this means that it skips testing 7.6 -> 7.7, which in fact would have failed if we tested it manually.
We could make sure that we catch this in future by complicating the test with checks for each possible starting version, but that seems slightly excessive.
Instead, I think we should just replace CurrentVersion in this line with {7,7}, since then we should actually end up testing all the upgrades correctly.
When we later add a new version, get_current_version would be changed, ensuring that this test would immediately fail when it can't find a clause of {7,7}, reminding us to add it.Let me know if you have any thoughts or better ideas.
Attachments
For Gerrit Dashboard: MB-62921 | ||||||
---|---|---|---|---|---|---|
# | Subject | Branch | Project | Status | CR | V |
213424,2 | MB-62921: Ensure unit test catches broken offline upgrade | master | ns_server | Status: MERGED | +2 | +1 |