-
Notifications
You must be signed in to change notification settings - Fork 55
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support HS upgrade tests #150
Comments
Firstly: I love the idea of running upgrade tests; I wish we had something like that for Synapse. Do you run it as part of your CI? To answer the question here: I agree that it does feel a bit feature-creepy; but given that Complement has most of the framework needed to make it happen, it seems like a bit of a shame for every HS impl to have to implement it from scratch. I think you can say much the same of Homerunner. Generally, having a collection of commands powered by the internals of Complement doesn't feel like a terrible thing to me. So basically, +1? |
Yeah CI runs the last published semver release and then the pushed commit over the top, as it takes a while to run every single release on CI all the time. This catches thinkos in upgrade scripts but isn't capable of catching everything as sometimes data corruption happens in earlier versions which then affects the next database migration. For example, dendrite 0.1 had corrupted state snapshots which then caused problems when those rooms made on 0.1 were migrated to HEAD. For that, we need to run everything from the beginning, which we'll just do as a pre-release checklist. I'll go ahead then with this and see what we end up with. |
Why should this be part of complement? Complement, in my eyes, about black-box compliance testing, there's nothing that the spec does to address, acknowledge, see, or even know about server upgrades, as that is a completely implementation-specific concept. What exactly should complement test for? That servers dont crash when they upgrade? How would that exactly be Complement's job to execute? Why can't that just be a standard external tool that's executed separately in a homeserver's CI pipeline instead? In my opinion, this is heavily feature creep, this should be its standalone tool, or banished to It could maybe be something along the lines of (calling it) |
Dendrite has https://github.com/matrix-org/dendrite/tree/master/cmd/dendrite-upgrade-tests which will automatically pull and run different versions of dendrite with the same database and ensure db migrations work correctly. It can be run for example by doing
./dendrite-upgrade-tests --from 0.1.0 --to 0.3.1
. This will:HEAD
).--to
.It looks like Synapse has some bisectability baked into the Complement Synapse image:
complement/dockerfiles/Synapse.Dockerfile
Line 14 in 76db9e5
If Complement standardised the format of this (e.g thou shalt accept the build arg
HS_VERSION=$semver
) then we could, in theory, make https://github.com/matrix-org/dendrite/tree/master/cmd/dendrite-upgrade-tests work for any HS (modulo working out how to do persistence, as currently Dendrite uses postgres and that assumption is baked into the upgrade testing infra: this could just be stating that the directory/data
is persisted across runs and you need to just dump your DB there).This arguably is and isn't feature creep. It is feature creep because it has nothing to do with the integration tests in
/tests
. It isn't feature creep because it is still ultimately testing HSes, just the upgrade paths.Thoughts @richvdh (on the basis that you added
SYNAPSE_VERSION
?The text was updated successfully, but these errors were encountered: