Skip to content

Conversation

@nexy7574
Copy link
Contributor

@nexy7574 nexy7574 commented Dec 22, 2025

Adds a test to prevent regressions fixed by element-hq/synapse#19321.

Reproduce element-hq/synapse#19120

Pull Request Checklist

doTestRestrictedRoomsLocalJoinNoCreatorsUsesPowerLevels(t, "12", "restricted")
}

func doTestRestrictedRoomsLocalJoinNoCreatorsUsesPowerLevels(t *testing.T, roomVersion string, joinRule string) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the reason other tests are written this way is so that it can be shared with tests/knock_restricted_test.go. I'm guessing we should also have knock_restricted test there.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(needs change)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added knock variants in 3cb69dd. Do you want them all to have docstrings or is TestRestrictedRoomsLocalJoinNoCreatorsUsesPowerLevelsV12's just being within view fine?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can say this:

// See docstring on `TestRestrictedRoomsLocalJoinNoCreatorsUsesPowerLevelsV12`

// an appropriate creator cannot be found.
//
// ref: https://github.com/element-hq/synapse/issues/19120
func TestRestrictedRoomsLocalJoinNoCreatorsUsesPowerLevels(t *testing.T) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking the time to write a test! It looks great overall 🤩

// an appropriate creator cannot be found.
//
// ref: https://github.com/element-hq/synapse/issues/19120
func TestRestrictedRoomsLocalJoinNoCreatorsUsesPowerLevels(t *testing.T) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tested to make sure it fails with the latest develop with Synapse and passes with element-hq/synapse#19321

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested again to make sure all of the new tests pass locally ✅ :

  1. Checkout Complement:
    1. Switch to the correct branch (this PR):
    2. git fetch origin refs/pull/836/head:pr-836
    3. git checkout pr-836
  2. Checkout Synapse:
    1. Switch to the correct branch (Fall back to checking power levels when sourcing local restricted join users element-hq/synapse#19321):
    2. git fetch origin refs/pull/19321/head:pr-19321
    3. git checkout pr-19321
  3. COMPLEMENT_DIR=../complement ./scripts-dev/complement.sh ./tests -run TestRestrictedRoomsLocalJoinNoCreatorsUsesPowerLevelsV
  4. COMPLEMENT_DIR=../complement ./scripts-dev/complement.sh ./tests -run TestKnockRestrictedRoomsLocalJoinNoCreatorsUsesPowerLevelsV

doTestRestrictedRoomsLocalJoinNoCreatorsUsesPowerLevels(t, "12", "restricted")
}

func doTestRestrictedRoomsLocalJoinNoCreatorsUsesPowerLevels(t *testing.T, roomVersion string, joinRule string) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(needs change)

doTestRestrictedRoomsLocalJoinNoCreatorsUsesPowerLevels(t, "12", "restricted")
}

func doTestRestrictedRoomsLocalJoinNoCreatorsUsesPowerLevels(t *testing.T, roomVersion string, joinRule string) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can say this:

// See docstring on `TestRestrictedRoomsLocalJoinNoCreatorsUsesPowerLevelsV12`

// Alice restricts the invite power level to moderators and promotes Bob to
// moderator.
state_key := ""
if roomVersion == "12" {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this works but doesn't feel the best given future room versions will come along. I guess we will leave the refactor until then. And most of these tests should be running across most of the room versions anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there's a more proper way to do it let me know, I'm used to something like mautrix's RoomVersion type which has feature flags, but I'm not familiar with complement's codebase all that much and I'm equally short on time so I haven't looked to see if there's a more appropriate solution

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There isn't a good strategy to address this kind of thing in Complement yet.

We can leave it to future refactors of the the tests that make them run across all room versions.

@MadLittleMods MadLittleMods merged commit 78c255e into matrix-org:main Jan 12, 2026
3 of 4 checks passed
MadLittleMods pushed a commit to element-hq/synapse that referenced this pull request Jan 12, 2026
…n users (#19321)

Fix #19120 by always falling
back to checking power levels for local users if a local creator cannot
be found in a v12 room.

Complement tests: matrix-org/complement#836
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants