Add in-repo Complement test to sanity check Synapse version matches git checkout #19476
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -705,6 +705,13 @@ jobs: | |
| toolchain: ${{ env.RUST_VERSION }} | ||
| - uses: Swatinem/rust-cache@779680da715d629ac1d338a641029a2f4372abb5 # v2.8.2 | ||
|
|
||
|
Contributor
Author
There was a problem hiding this comment.
Yep. Really easy to do when anything in backticks is executed and you like to format with markdown. Pushed it anyway ⏩ |
||
| # We use `poetry` in `complement.sh` | ||
| - uses: matrix-org/setup-python-poetry@5bbf6603c5c930615ec8a29f1b5d7d258d905aa4 # v2.0.0 | ||
| with: | ||
| poetry-version: "2.1.1" | ||
| # Matches the `path` where we checkout Synapse above | ||
| working-directory: "synapse" | ||
|
|
||
| - name: Prepare Complement's Prerequisites | ||
| run: synapse/.ci/scripts/setup_complement_prerequisites.sh | ||
|
|
||
|
|
@@ -713,6 +720,32 @@ jobs: | |
| cache-dependency-path: complement/go.sum | ||
|
Contributor
Author
There was a problem hiding this comment. Putting this PR up for review to get help with the problem summarized in
Contributor
Author
There was a problem hiding this comment. This PR is purposely not up to date with the
Contributor
Author
There was a problem hiding this comment. lol, trying to get this new test to fail in CI but it looks like GitHub Versus the previous GitHub Looks like this was reverted in actions/runner-images#13708 |
||
| go-version-file: complement/go.mod | ||
|
|
||
| # Run the image sanity check test first as this is the first thing we want to know | ||
| # about (are we actually testing what we expect?) and we don't want to debug | ||
| # downstream failures (wild goose chase). | ||
| - name: Sanity check Complement image | ||
| id: run_sanity_check_complement_image_test | ||
| # -p=1: We're using `-p 1` to force the test packages to run serially as GHA boxes | ||
| # are underpowered and don't like running tons of Synapse instances at once. | ||
| # -json: Output JSON format so that gotestfmt can parse it. | ||
| # | ||
| # tee /tmp/gotest-complement.log: We tee the output to a file so that we can re-process it | ||
| # later on for better formatting with gotestfmt. But we still want the command | ||
| # to output to the terminal as it runs so we can see what's happening in | ||
| # real-time. | ||
| run: | | ||
| set -o pipefail | ||
| COMPLEMENT_DIR=`pwd`/complement synapse/scripts-dev/complement.sh --in-repo -p 1 -json -run 'TestSynapseVersion/Synapse_version_matches_current_git_checkout' 2>&1 | tee /tmp/gotest-sanity-check-complement.log | ||
|
Contributor
Author
There was a problem hiding this comment. With Since golang/go#64500 was rejected, best we could do is grep over the output for Probably just move on without this ⏩ As a final stop-gap, this test will also be run as part of the normal |
||
| shell: bash | ||
| env: | ||
| POSTGRES: ${{ (matrix.database == 'Postgres') && 1 || '' }} | ||
| WORKERS: ${{ (matrix.arrangement == 'workers') && 1 || '' }} | ||
|
|
||
| - name: Formatted sanity check Complement test logs | ||
| # Always run this step if we attempted to run the Complement tests. | ||
| if: always() && steps.run_sanity_check_complement_image_test.outcome != 'skipped' | ||
| run: cat /tmp/gotest-sanity-check-complement.log | gotestfmt -hide "successful-downloads,empty-packages" | ||
|
|
||
|
Contributor
Author
There was a problem hiding this comment. Before I update this branch with the latest But CI won't run for some reason. I assume that I have a syntax error in my update here but how do I find it? I thought GitHub would tell me somewhere
Contributor
Author
There was a problem hiding this comment. #19532 has the exact same CI but it runs there 🤔 I feel like this has something to do with the conflict/outdated but I don't want to update for the reason above.
Contributor
Author
There was a problem hiding this comment. Seems to run when I manually trigger the workflow (from this page) for this branch, https://github.com/element-hq/synapse/actions/runs/22733335657 And we can actually see an error for the
Contributor
Author
There was a problem hiding this comment. (got the sanity check test running, see commits below) Still don't know why CI isn't triggering on its own in this PR.
Contributor
Author
There was a problem hiding this comment.
Found some evidence of this in https://github.com/orgs/community/discussions/11265 And usually when I naturally come across this situation, I just update the branch and things work. But in this case, I didn't want to immediately do it so we just end up stuck with no feedback
Contributor
There was a problem hiding this comment. Yeah, it's both intuitive and not at the same time. The CI runs against the result of merging the PR into the target branch (good so that you get the most accurate thing being tested). They should really add a warning when there are conflicts present that this blocks CI... |
||
| - name: Run Complement Tests | ||
| id: run_complement_tests | ||
| # -p=1: We're using `-p 1` to force the test packages to run serially as GHA boxes | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Add in-repo Complement test to sanity check Synapse version matches git checkout (testing what we think we are). |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,101 @@ | ||
| // This file is licensed under the Affero General Public License (AGPL) version 3. | ||
| // | ||
| // Copyright (C) 2026 Element Creations Ltd | ||
| // | ||
| // This program is free software: you can redistribute it and/or modify | ||
| // it under the terms of the GNU Affero General Public License as | ||
| // published by the Free Software Foundation, either version 3 of the | ||
| // License, or (at your option) any later version. | ||
| // | ||
| // See the GNU Affero General Public License for more details: | ||
| // <https://www.gnu.org/licenses/agpl-3.0.html>. | ||
|
|
||
| package synapse_tests | ||
|
|
||
| import ( | ||
| "net/http" | ||
| "os/exec" | ||
| "strings" | ||
| "testing" | ||
|
|
||
| "github.com/matrix-org/complement" | ||
| "github.com/matrix-org/complement/match" | ||
| "github.com/matrix-org/complement/must" | ||
| "github.com/tidwall/gjson" | ||
| ) | ||
|
|
||
| func TestSynapseVersion(t *testing.T) { | ||
| deployment := complement.Deploy(t, 1) | ||
| defer deployment.Destroy(t) | ||
|
|
||
| unauthedClient := deployment.UnauthenticatedClient(t, "hs1") | ||
|
|
||
| // Sanity check that the version of Synapse used in the `COMPLEMENT_BASE_IMAGE` | ||
| // matches the same git commit we have checked out. This ensures that the image being | ||
| // used in Complement is the one that we just built locally with `complement.sh` | ||
| // instead of accidentally pulling in some remote one. | ||
| // | ||
| // This test is expected to pass if you use `complement.sh`. | ||
| // | ||
| // If this test fails, it probably means that Complement is using an image that | ||
| // doesn't encompass the changes you have checked out (unexpected). We want to yell | ||
| // loudly and point out what's wrong instead of silently letting your PRs pass | ||
| // without actually being tested. | ||
| t.Run("Synapse version matches current git checkout", func(t *testing.T) { | ||
| // Get the Synapse version details of the current git checkout | ||
| checkoutSynapseVersion := runCommand( | ||
| t, | ||
| []string{ | ||
| "poetry", | ||
|
Contributor
There was a problem hiding this comment. I guess it's not necessarily a given that people will have poetry if our
Contributor
Author
There was a problem hiding this comment. I would expect this to be updated to whatever our developer flow is ⏩ |
||
| "run", | ||
| "python", | ||
| "-c", | ||
| "from synapse.util import SYNAPSE_VERSION; print(SYNAPSE_VERSION)", | ||
| }, | ||
| ) | ||
|
|
||
| // Find the version details of the Synapse instance deployed from the Docker image | ||
| res := unauthedClient.MustDo(t, "GET", []string{"_matrix", "federation", "v1", "version"}) | ||
|
MadLittleMods marked this conversation as resolved.
|
||
| body := must.MatchResponse(t, res, match.HTTPResponse{ | ||
| StatusCode: http.StatusOK, | ||
| JSON: []match.JSON{ | ||
| match.JSONKeyPresent("server"), | ||
| }, | ||
| }) | ||
| rawSynapseVersionString := gjson.GetBytes(body, "server.version").Str | ||
| t.Logf( | ||
| "Synapse version string from federation version endpoint: %s", | ||
| rawSynapseVersionString, | ||
| ) | ||
|
|
||
| must.Equal( | ||
| t, | ||
| rawSynapseVersionString, | ||
| checkoutSynapseVersion, | ||
| "Synapse version in the checkout doesn't match the Synapse version that Complement is running. "+ | ||
| "If this test fails, it probably means that Complement is using an image that "+ | ||
| "doesn't encompass the changes you have checked out (unexpected). We want to yell "+ | ||
| "loudly and point out what's wrong instead of silently letting your PRs pass "+ | ||
| "without actually being tested.", | ||
| ) | ||
| }) | ||
| } | ||
|
|
||
| // runCommand will run the given command and return the stdout (whitespace | ||
| // trimmed). | ||
| func runCommand(t *testing.T, commandPieces []string) string { | ||
| t.Helper() | ||
|
|
||
| // Then run our actual command | ||
| cmd := exec.Command(commandPieces[0], commandPieces[1:]...) | ||
| output, err := cmd.Output() | ||
|
Contributor
There was a problem hiding this comment. Consider
Contributor
Author
There was a problem hiding this comment. I think that's expected as we just want the machine-readable/pipeable values from Although I don't think it matters in this case with the command we're using. Ideally, we'd print everything when the command fails 👍 |
||
| if err != nil { | ||
| t.Fatalf( | ||
| "runCommand: failed to run command (%s): %v", | ||
| strings.Join(commandPieces, " "), | ||
| err, | ||
| ) | ||
| } | ||
|
|
||
| return strings.TrimSpace(string(output)) | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could also copy these changes to
.github/workflows/latest_deps.ymland.github/workflows/twisted_trunk.ymlwhere we also run Complement tests.We already have the
--in-repovariant there so the sanity check test will run as part of that. We just won't have the initial feedback of this first run like we have here in.github/workflows/tests.ymlCan easily copy across if you poke. Ideally, all of this boilerplate would be shared as @anoadragon453 suggested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Making a re-usable Complement workflow in #19533