Add in-repo Complement test to sanity check Synapse version matches git checkout#19476
Conversation
| @@ -707,28 +707,28 @@ jobs: | |||
| cache-dependency-path: complement/go.sum | |||
There was a problem hiding this comment.
This PR is purposely not up to date with the develop at the moment so I can still reproduce the problem from #19475
There was a problem hiding this comment.
lol, trying to get this new test to fail in CI but it looks like GitHub Current runner version: '2.332.0' is back to using Docker Client/Server 28.0.4 (source) which doesn't have the problem.
Versus the previous GitHub Current runner version: '2.331.0' we saw in #19475 which was using Docker Client/Server 29.1.5 (source)
Looks like this was reverted in actions/runner-images#13708
reivilibre
left a comment
There was a problem hiding this comment.
Seems like a good idea given it bit us, but I guess we need a workaround for the egg info. (Maybe rm it opportunistically in the complement.sh script? Might be our best option)
|
|
||
| // Then run our actual command | ||
| cmd := exec.Command(commandPieces[0], commandPieces[1:]...) | ||
| output, err := cmd.Output() |
There was a problem hiding this comment.
Consider cmd.CombinedOutput() to get stderr as well. Currently I think stderr output would go uncaptured?
There was a problem hiding this comment.
I think that's expected as we just want the machine-readable/pipeable values from stdout
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 👍
| checkoutSynapseVersion := runCommand( | ||
| t, | ||
| []string{ | ||
| "poetry", |
There was a problem hiding this comment.
I guess it's not necessarily a given that people will have poetry if our uv plans pick up any momentum at all. However I'm also happy to leave this easy fix for when it's actually a problem.
There was a problem hiding this comment.
I would expect this to be updated to whatever our developer flow is ⏩
| # available in the Docker image for Synapse to generate from. | ||
| ARG SYNAPSE_VERSION_STRING | ||
| # Pass it through to Synapse as an environment variable. | ||
| ENV SYNAPSE_VERSION_STRING=${SYNAPSE_VERSION_STRING} |
There was a problem hiding this comment.
(had to look up that this is actually optional, seems like it is)
| # We don't run `gotestfmt` for the sanity check step as it's only one test so | ||
| # there is nothing to organize. And the extra step is more noise to click | ||
| # through and grok. If this step fails, it will be obvious still. | ||
|
|
There was a problem hiding this comment.
Before I update this branch with the latest develop, I want to see it fail.
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
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.
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 complement job ❌:
synapse/scripts-dev/complement.sh: line 199: poetry: command not found
Error: Process completed with exit code 127.
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.
There was a problem hiding this comment.
I feel like this has something to do with the conflict/outdated but I don't want to update for the reason above.
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 ![]()
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...
Usage:
command [options] [arguments]
Options:
-h, --help Display help for the given command. When no command is given display help for the list command.
-q, --quiet Do not output any message.
-V, --version Display this application version.
--ansi Force ANSI output.
--no-ansi Disable ANSI output.
-n, --no-interaction Do not ask any interactive question.
--no-plugins Disables plugins.
--no-cache Disables Poetry source caches.
-P, --project=PROJECT Specify another path as the project root. All command-line arguments will be resolved relative to the current working directory.
-C, --directory=DIRECTORY The working directory for the Poetry command (defaults to the current working directory). All command-line arguments will be resolved relative to the given directory.
-v|vv|vvv, --verbose Increase the verbosity of messages: 1 for normal output, 2 for more verbose output and 3 for debug.
Available commands:
about Shows information about Poetry.
add Adds a new dependency to pyproject.toml and installs it.
build Builds a package, as a tarball and a wheel by default.
check Validates the content of the pyproject.toml file and its consistency with the poetry.lock file.
config Manages configuration settings.
export Exports the lock file to alternative formats.
help Displays help for a command.
init Creates a basic pyproject.toml file in the current directory.
install Installs the project dependencies.
list Lists commands.
lock Locks the project dependencies.
new Creates a new Python project at <path>.
publish Publishes a package to a remote repository.
remove Removes a package from the project dependencies.
run Runs a command in the appropriate environment.
search Searches for packages on remote repositories.
show Shows information about packages.
sync Update the project's environment according to the lockfile.
update Update the dependencies as according to the pyproject.toml file.
version Shows the version of the project or bumps it when a valid bump rule is provided.
cache
cache clear Clears a Poetry cache by name.
cache list List Poetry's caches.
debug
debug info Shows debug information.
debug resolve Debugs dependency resolution.
debug tags Shows compatible tags for your project's current active environment.
env
env activate Print the command to activate a virtual environment.
env info Displays information about the current environment.
env list Lists all virtualenvs associated with the current project.
env remove Remove virtual environments associated with the project.
env use Activates or creates a new virtualenv for the current project.
python
python install Install the specified Python version from the Python Standalone Builds project. (experimental feature)
python list Shows Python versions available for this environment. (experimental feature)
python remove Remove the specified Python version if managed by Poetry. (experimental feature)
self
self add Add additional packages to Poetry's runtime environment.
self install Install locked packages (incl. addons) required by this Poetry installation.
self lock Lock the Poetry installation's system requirements.
self remove Remove additional packages from Poetry's runtime environment.
self show Show packages from Poetry's runtime environment.
self show plugins Shows information about the currently installed plugins.
self sync Sync Poetry's own environment according to the locked packages (incl. addons) required by this Poetry installation.
self update Updates Poetry to the latest version.
source
source add Add source configuration for project.
source remove Remove source configured for the project.
source show Show information about sources configured for the project. available
Alternatively, we could avoid the JSON
| # 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 |
There was a problem hiding this comment.
With go test, I wish there was a way to enforce that some test is actually run with this -run '...' filter.
Since golang/go#64500 was rejected, best we could do is grep over the output for no tests to run. But it also gets slightly more tricky as no tests to run will be printed for the other packages that don't have a matching test.
Probably just move on without this ⏩
As a final stop-gap, this test will also be run as part of the normal --in-repo run below. So at-least things won't be able to accidentally pass if this test name changes from under us. Running it first here, just gives us the proper feedback on the first source of wrong-ness.
| @@ -699,6 +699,13 @@ jobs: | |||
| toolchain: ${{ env.RUST_VERSION }} | |||
| - uses: Swatinem/rust-cache@779680da715d629ac1d338a641029a2f4372abb5 # v2.8.2 | |||
|
|
|||
There was a problem hiding this comment.
We could also copy these changes to .github/workflows/latest_deps.yml and .github/workflows/twisted_trunk.yml where we also run Complement tests.
We already have the --in-repo variant 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.yml
Can easily copy across if you poke. Ideally, all of this boilerplate would be shared as @anoadragon453 suggested.
There was a problem hiding this comment.
Making a re-usable Complement workflow in #19533
Conflicts: scripts-dev/complement.sh
reivilibre
left a comment
There was a problem hiding this comment.
(The commit message on 296ba32 is funny, did you accidentally let poetry get expanded by your shell? :)))
Anyway this seems good to me, thanks for putting in the effort!
| # We don't run `gotestfmt` for the sanity check step as it's only one test so | ||
| # there is nothing to organize. And the extra step is more noise to click | ||
| # through and grok. If this step fails, it will be obvious still. | ||
|
|
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...
| @@ -705,6 +705,13 @@ jobs: | |||
| toolchain: ${{ env.RUST_VERSION }} | |||
| - uses: Swatinem/rust-cache@779680da715d629ac1d338a641029a2f4372abb5 # v2.8.2 | |||
|
|
|||
There was a problem hiding this comment.
(The commit message on 296ba32 is funny, did you accidentally let
poetryget expanded by your shell? :)))
Yep. Really easy to do when anything in backticks is executed and you like to format with markdown. Pushed it anyway ⏩
|
@reivilibre Thanks for the review and help figuring out the Python packaging puzzles 😵 |
…omplement.sh` (#19578) ❌ `Build and push complement image`, https://github.com/element-hq/synapse/actions/runs/23176317296/job/67339146082 ``` scripts-dev/complement.sh: line 227: poetry: command not found ``` Follow-up to #19523 This regressed in #19476 ### Testing strategy 1. Visit https://github.com/element-hq/synapse/actions/workflows/push_complement_image.yml 1. **Run workflow**: - **Use workflow from:** `madlittlemods/fix-complement-push-image-ci-job-poetry` - **Branch:** `develop` 1. Wait for CI to run and pass ✅
This PR contains the following updates: | Package | Update | Change | |---|---|---| | [element-hq/synapse](https://github.com/element-hq/synapse) | minor | `v1.149.1` → `v1.150.0` | --- ### Release Notes <details> <summary>element-hq/synapse (element-hq/synapse)</summary> ### [`v1.150.0`](https://github.com/element-hq/synapse/releases/tag/v1.150.0) [Compare Source](element-hq/synapse@v1.149.1...v1.150.0) ### Synapse 1.150.0 (2026-03-24) No significant changes since 1.150.0rc1. ### Synapse 1.150.0rc1 (2026-03-17) #### Features - Add experimental support for the [MSC4370](matrix-org/matrix-spec-proposals#4370) Federation API `GET /extremities` endpoint. ([#​19314](element-hq/synapse#19314)) - [MSC4140: Cancellable delayed events](matrix-org/matrix-spec-proposals#4140): When persisting a delayed event to the timeline, include its `delay_id` in the event's `unsigned` section in `/sync` responses to the event sender. ([#​19479](element-hq/synapse#19479)) - Expose [MSC4354 Sticky Events](matrix-org/matrix-spec-proposals#4354) over the legacy (v3) /sync API. ([#​19487](element-hq/synapse#19487)) - When Matrix Authentication Service (MAS) integration is enabled, allow MAS to set the user locked status in Synapse. ([#​19554](element-hq/synapse#19554)) #### Bugfixes - Fix `Build and push complement image` CI job pointing to non-existent image. ([#​19523](element-hq/synapse#19523)) - Fix a bug introduced in v1.26.0 that caused deactivated, erased users to not be removed from the user directory. ([#​19542](element-hq/synapse#19542)) #### Improved Documentation - In the Admin API documentation, always express path parameters as `/<param>` instead of as `/$param`. ([#​19307](element-hq/synapse#19307)) - Update docs to clarify `outbound_federation_restricted_to` can also be used with the [Secure Border Gateway (SBG)](https://element.io/en/server-suite/secure-border-gateways). ([#​19517](element-hq/synapse#19517)) - Unify Complement developer docs. ([#​19518](element-hq/synapse#19518)) #### Internal Changes - Put membership updates in a background resumable task when changing the avatar or the display name. ([#​19311](element-hq/synapse#19311)) - Add in-repo Complement test to sanity check Synapse version matches git checkout (testing what we think we are). ([#​19476](element-hq/synapse#19476)) - Migrate `dev` dependencies to [PEP 735](https://peps.python.org/pep-0735/) dependency groups. ([#​19490](element-hq/synapse#19490)) - Remove the optional `systemd-python` dependency and the `systemd` extra on the `synapse` package. ([#​19491](element-hq/synapse#19491)) - Avoid re-computing the event ID when cloning events. ([#​19527](element-hq/synapse#19527)) - Allow caching of the `/versions` and `/auth_metadata` public endpoints. ([#​19530](element-hq/synapse#19530)) - Add a few labels to the number groupings in the `Processed request` logs. ([#​19548](element-hq/synapse#19548)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My44NC4yIiwidXBkYXRlZEluVmVyIjoiNDMuODQuMiIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiaW1hZ2UiXX0=--> Reviewed-on: https://gitea.alexlebens.dev/alexlebens/infrastructure/pulls/5040 Co-authored-by: Renovate Bot <renovate-bot@alexlebens.net> Co-committed-by: Renovate Bot <renovate-bot@alexlebens.net>
Add in-repo Complement test to sanity check Synapse version matches git checkout
This way we actually detect problems like #19475 as they happen instead of being invisible until something breaks.
Sanity check that Complement is testing against your code changes (whether it be local or from the PR in CI).
Dev notes
Synapse version string generated by https://github.com/matrix-org/matrix-python-common/blob/4084b21af839c50f775447d02ca4f1854e2e6191/src/matrix_common/versionstring.py
Pull Request Checklist
EventStoretoEventWorkerStore.".code blocks.