Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert more tests to Complement #353

Closed
wants to merge 13 commits into from
Closed

Convert more tests to Complement #353

wants to merge 13 commits into from

Conversation

S7evinK
Copy link
Contributor

@S7evinK S7evinK commented Mar 29, 2022

Again adds new tests:
tests/32room-versions.pl - this seems to slow down the tests quite a bit (at least locally)
tests/10apidoc/36room-levels.pl
tests/30rooms/01state.pl

defer func() {
// sytest: Both GET and PUT work
if successCount != 2 {
t.Fatalf("expected GET and PUT to work")
Copy link
Member

Choose a reason for hiding this comment

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

Why is this necessary? t.Run("Parallel", ... will block until the tests complete. Each test can fail the subtest which will have a knock-on effect and fail the outer test, so this seems entirely redundant? We never do this anywhere in Complement AFAICT.


powerLevels := gjson.ParseBytes(must.ParseJSON(t, res.Body))
changedUser := client.GjsonEscape("@random-other-user:their.home")
alicePowerLevel := powerLevels.Get("users." + alice.UserID).Int()
Copy link
Member

Choose a reason for hiding this comment

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

Technically alice.UserID should be gjson escaped as well.

reqBody = client.WithJSONBody(t, map[string]interface{}{})
alice.MustDoFunc(t, "PUT", []string{"_matrix", "client", "v3", "rooms", roomID, "state", "m.room.power_levels"}, reqBody)
// this should now give a 403 (not a 500)
res := alice.DoFunc(t, "PUT", []string{"_matrix", "client", "v3", "rooms", roomID, "state", "m.room.power_levels"}, reqBody)
Copy link
Member

Choose a reason for hiding this comment

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

This isn't what the sytest is doing.

      # absence of an 'events' key
      matrix_put_room_state(
         $user,
         $room_id,
         type      => "m.room.power_levels",
         state_key => "",
         content   => {
            users => {
               $user->user_id => 100,
            },
         },
      )->then( sub {
         # absence of a 'users' key
         matrix_put_room_state(
            $user,
            $room_id,
            type      => "m.room.power_levels",
            state_key => "",
            content   => {
            },
         );
      })->then( sub {
         # this should now give a 403 (not a 500)
         matrix_put_room_state(
            $user,
            $room_id,
            type      => "m.room.power_levels",
            state_key => "",
            content   => {
               users => {},
            },
         ) -> main::expect_http_403;
      })

So the order is:

  • PUT with users key with alice
  • PUT with empty content
  • PUT with users key present but empty -> 403

})
}

func joinRoomSynced(t *testing.T, cl *client.CSAPI, roomID, alias string) {
Copy link
Member

Choose a reason for hiding this comment

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

These should probably go into the Client impl to be honest. For now leave them here though.

roomID := createRoomSynced(t, alice, map[string]interface{}{
"room_version": roomVersion,
})
alice.MustSyncUntil(t, client.SyncReq{}, client.SyncJoinedTo(alice.UserID, roomID))
Copy link
Member

Choose a reason for hiding this comment

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

Isn't the whole point of the createRoomSynced function to literally do this? Why do it again?

}
for typ, joiner := range userTypes {
typ := typ
joiner := joiner
Copy link
Member

Choose a reason for hiding this comment

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

Need comments to explain why you do this (else it'll take the last value only).

})
charlie.MustSyncUntil(t, client.SyncReq{}, client.SyncInvitedTo(charlie.UserID, roomID))
charlie.LeaveRoom(t, roomID)
})
Copy link
Member

Choose a reason for hiding this comment

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

No check for alice to see that the invite was rejected?

@S7evinK S7evinK requested a review from kegsay April 11, 2022 08:40
…roomversions

# Conflicts:
#	dockerfiles/synapse/homeserver.yaml
#	dockerfiles/synapse/workers-shared.yaml
#	tests/csapi/rooms_state_test.go
@S7evinK S7evinK requested review from a team as code owners May 30, 2023 13:52
@S7evinK S7evinK removed request for a team and kegsay May 30, 2023 13:54
@S7evinK S7evinK marked this pull request as draft May 30, 2023 13:54
@kegsay kegsay added the stale This issue or PR is old and may be closed soon label Sep 11, 2023
@kegsay
Copy link
Member

kegsay commented Sep 6, 2024

Closing as stale.

@kegsay kegsay closed this Sep 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale This issue or PR is old and may be closed soon
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants