Add TestRestrictedRoomsLocalJoinNoCreatorsUsesPowerLevels#836
Add TestRestrictedRoomsLocalJoinNoCreatorsUsesPowerLevels#836MadLittleMods merged 8 commits intomatrix-org:mainfrom
TestRestrictedRoomsLocalJoinNoCreatorsUsesPowerLevels#836Conversation
Signed-off-by: timedout <git@nexy7574.co.uk>
| doTestRestrictedRoomsLocalJoinNoCreatorsUsesPowerLevels(t, "12", "restricted") | ||
| } | ||
|
|
||
| func doTestRestrictedRoomsLocalJoinNoCreatorsUsesPowerLevels(t *testing.T, roomVersion string, joinRule string) { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Added knock variants in 3cb69dd. Do you want them all to have docstrings or is TestRestrictedRoomsLocalJoinNoCreatorsUsesPowerLevelsV12's just being within view fine?
There was a problem hiding this comment.
We can say this:
// See docstring on `TestRestrictedRoomsLocalJoinNoCreatorsUsesPowerLevelsV12`
tests/restricted_rooms_test.go
Outdated
| // an appropriate creator cannot be found. | ||
| // | ||
| // ref: https://github.com/element-hq/synapse/issues/19120 | ||
| func TestRestrictedRoomsLocalJoinNoCreatorsUsesPowerLevels(t *testing.T) { |
There was a problem hiding this comment.
Thanks for taking the time to write a test! It looks great overall 🤩
tests/restricted_rooms_test.go
Outdated
| // an appropriate creator cannot be found. | ||
| // | ||
| // ref: https://github.com/element-hq/synapse/issues/19120 | ||
| func TestRestrictedRoomsLocalJoinNoCreatorsUsesPowerLevels(t *testing.T) { |
There was a problem hiding this comment.
I've tested to make sure it fails with the latest develop with Synapse and passes with element-hq/synapse#19321 ✅
There was a problem hiding this comment.
Tested again to make sure all of the new tests pass locally ✅ :
- Checkout Complement:
- Switch to the correct branch (this PR):
git fetch origin refs/pull/836/head:pr-836git checkout pr-836
- Checkout Synapse:
- Switch to the correct branch (Fall back to checking power levels when sourcing local restricted join users element-hq/synapse#19321):
git fetch origin refs/pull/19321/head:pr-19321git checkout pr-19321
COMPLEMENT_DIR=../complement ./scripts-dev/complement.sh ./tests -run TestRestrictedRoomsLocalJoinNoCreatorsUsesPowerLevelsVCOMPLEMENT_DIR=../complement ./scripts-dev/complement.sh ./tests -run TestKnockRestrictedRoomsLocalJoinNoCreatorsUsesPowerLevelsV
Co-authored-by: Eric Eastwood <madlittlemods@gmail.com>
Signed-off-by: timedout <git@nexy7574.co.uk>
| doTestRestrictedRoomsLocalJoinNoCreatorsUsesPowerLevels(t, "12", "restricted") | ||
| } | ||
|
|
||
| func doTestRestrictedRoomsLocalJoinNoCreatorsUsesPowerLevels(t *testing.T, roomVersion string, joinRule string) { |
Signed-off-by: timedout <git@nexy7574.co.uk>
Signed-off-by: timedout <git@nexy7574.co.uk>
Signed-off-by: timedout <git@nexy7574.co.uk>
| doTestRestrictedRoomsLocalJoinNoCreatorsUsesPowerLevels(t, "12", "restricted") | ||
| } | ||
|
|
||
| func doTestRestrictedRoomsLocalJoinNoCreatorsUsesPowerLevels(t *testing.T, roomVersion string, joinRule string) { |
There was a problem hiding this comment.
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" { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Judging from the changes in #840, it looks like this would be possible with iRoomVer := gomatrixserverlib.MustGetRoomVersion(roomVer) -> iRoomVer.DomainlessRoomIDs()
Signed-off-by: timedout <git@nexy7574.co.uk>
…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
Adds a test to prevent regressions fixed by element-hq/synapse#19321.
Reproduce element-hq/synapse#19120
Pull Request Checklist