Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

./scripts-dev/complement.sh and GHA complement job diverged #11363

Closed
Bubu opened this issue Nov 16, 2021 · 5 comments · Fixed by #11389
Closed

./scripts-dev/complement.sh and GHA complement job diverged #11363

Bubu opened this issue Nov 16, 2021 · 5 comments · Fixed by #11389
Assignees
Labels
T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks.

Comments

@Bubu
Copy link
Contributor

Bubu commented Nov 16, 2021

It seems like 684d19a added the msc2716 tag to the complement.sh script but it seems that was never added to the GHA workflow here: https://github.com/matrix-org/synapse/blob/develop/.github/workflows/tests.yml#L377

Consequently that (those?) tests are now failing on develop.

@Bubu Bubu changed the title ./scripts-dev/complement.sh and GHA complement job ./scripts-dev/complement.sh and GHA complement job diverged Nov 16, 2021
@clokep
Copy link
Member

clokep commented Nov 17, 2021

@MadLittleMods Do you know if those tests should be passing or not?

@clokep clokep added the T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. label Nov 17, 2021
@MadLittleMods
Copy link
Contributor

MadLittleMods commented Nov 17, 2021

I think they would pass but it looks like the Complement tests are using the org.matrix.msc2716v4 room version which hasn't merged to Synapse yet (potentially merged in #11243 or #9445 or not merged at all #11243 (comment)). Feel free to remove, it's always a juggle trying to add it back when developing a new MSC though.

I don't think the tests are running in CI (GHA) because the command is separate from complement.sh though?

@clokep
Copy link
Member

clokep commented Nov 17, 2021

I don't think the tests are running in CI (GHA) because the command is separate from complement.sh though?

Right, but for people developing against Synapse it is frequent to manually run complement commands with this script, so it is best to keep it in a state where it is expected to pass.

@DMRobertson
Copy link
Contributor

Is this fixed by #11368?

@clokep
Copy link
Member

clokep commented Nov 18, 2021

Is this fixed by #11368

No, I'll do a PR to fix it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants