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

Add tests for MSC2716 and backfilling history #68

Merged
merged 93 commits into from
Jul 15, 2021

Conversation

MadLittleMods
Copy link
Contributor

@MadLittleMods MadLittleMods commented Jan 28, 2021

This PR is a follow-up to #74 which adds support for application services in Complement blueprints.

Add tests for MSC2716 and backfilling history

Dev notes

contextRes := alice.MustDoFunc(t, "GET", []string{"_matrix", "client", "r0", "rooms", roomID, "context", eventStar}, client.WithContentType("application/json"), client.WithQueries(url.Values{
	"limit": []string{"100"},
}))
contextResBody := client.ParseJSON(t, contextRes)
logrus.WithFields(logrus.Fields{
	"contextResBody": string(contextResBody),
}).Error("context res")
Previous code before functional client.go
contextRes := alice.MustDoRaw(t, "GET", []string{"_matrix", "client", "r0", "rooms", roomID, "context", eventStar}, nil, "application/json", url.Values{
	"limit": []string{"100"},
})
contextResBody := client.ParseJSON(t, contextRes)
logrus.WithFields(logrus.Fields{
	"contextResBody": string(contextResBody),
}).Error("context res")

Always show server logs even if the test passes: Change this line in internal/docker/deployment.go#L34 so it always passes true:

--- a/internal/docker/deployment.go
+++ b/internal/docker/deployment.go
@@ -31,7 +31,7 @@ type HomeserverDeployment struct {
 // will print container logs before killing the container.
 func (d *Deployment) Destroy(t *testing.T) {
        t.Helper()
-       d.Deployer.Destroy(d, t.Failed())
+       d.Deployer.Destroy(d, true) // t.Failed()
 }

Create random string:

hsTokenBytes := make([]byte, 32)
_, err := rand.Read(hsTokenBytes)
if err != nil {
	return bp, err
}

var hsToken string = hex.EncodeToString(hsTokenBytes)

Make it possible to read the response body multiple times

body := client.ParseJSON(t, res)
// Since the original body can only be read once, create a new one from the body bytes we just read
res.Body = ioutil.NopCloser(bytes.NewBuffer(body))

// ready to read the body again from res

Hook up Element (Matrix client) to the Complement server after a test runs

Because PublishAllPorts: true, is set when the container is created, the homeserver is available to point a Matrix client against.

  1. In your test comment out defer deployment.Destroy(t) and replace with defer time.Sleep(2 * time.Hour) to keep the homeserver running after the tests complete
  2. Start the Complement tests (./scripts-dev/complement.sh)
  3. Save the Element config as /Users/eric/Downloads/riot-complement-config.json and replace the port with the port you see from docker ps (docker ps -f name=complement_ to just filter to the Complement containers)
    {
      "default_server_config": {
        "m.homeserver": {
          "base_url": "http://localhost:55449",
          "server_name": "my.complement.host"
        }
      },
      "brand": "Element"
    }
  4. Start up Element (your matrix client)
    docker run -it --rm \
        --publish 7080:80 \
        --volume /Users/eric/Downloads/riot-complement-config.json:/app/config.json \
        --name riot-complement \
        vectorim/riot-web:v1.7.8
    
  5. Now you can visit http://localhost:7080/ and register a new user and explore the rooms from the test output

Access database in Complement test Docker container:

$ docker exec -it complement_1_hs_with_application_service.hs1_2 /bin/bash

$ apt-get update && apt-get install -y sqlite3

$ sqlite3
> .open /conf/homeserver.db

TODO

  • Define the client as an appservice
  • Add tests for healing state, https://github.com/matrix-org/matrix-doc/pull/2716/files#r559846587
  • Add test to ensure only appservice can use the ?prev_event query param
  • Add test for "If an unrecognised event ID is specified as a prev_event, the request fails with a 404."
  • Add test for "An event must not have more than 20 prev_events."
    • This is enforced by Synapse but would be fine to add a test. Can add in another PR
  • Can we support historical state events as the spec wants? PUT /_matrix/client/r0/rooms/{roomId}/state/{eventType}/{stateKey}
    • We've moved to a batch send endpoint
  • Add test for inserting historical messages in a room from a user that isn't already in the room
    • I suppose this should be a federation test and testing /backfill essentially
    • Need to make sure the join and membership events are there
    • Make sure avatar and displayName work
    • As an application service, can use the ?user_id query parameter to act as a virtual user
  • Add federation test where hs2 is already joined to the room, historical messages are added, scrollback shows the messages
  • Add federation test where hs2 is already joined to the room and has backfilled some historical messages. Then more historical messages are added and scrollback is able to show the new historical messages
  • Would be cool to make a generalized solution to make a TARDIS visualization for any test run in Complement (after hook to save a file which we can view in TARDIS)
  • Create PR to define namespace regexes, Add tests for MSC2716 and backfilling history #68 (comment) -> Allow application service YAML to be customised #148

@MadLittleMods MadLittleMods force-pushed the eric/msc2716-backfilling-history branch from f620354 to 15ba451 Compare January 28, 2021 07:58
@MadLittleMods MadLittleMods force-pushed the eric/msc2716-backfilling-history branch from bd5b0b8 to dabdf5a Compare January 30, 2021 04:25
tests/msc2716_test.go Outdated Show resolved Hide resolved
MadLittleMods added a commit to MadLittleMods/tardis that referenced this pull request Feb 2, 2021
Edits to make TARDIS work with Synapse while writing Complement tests for [MSC 2716](matrix-org/matrix-spec-proposals#2716).

 - matrix-org/synapse#9247
 - matrix-org/complement#68
MadLittleMods added a commit to MadLittleMods/tardis that referenced this pull request Feb 2, 2021
README.md Outdated Show resolved Hide resolved
Before:

```
=== CONT  TestBackfillingHistory
    client.go:356: GET /_matrix/client/r0/sync => 200 OK (31.496008ms)
    client.go:356: POST /_matrix/client/unstable/org.matrix.msc2716/rooms/!GkmAGvcDmllsuqLgZA:hs1/batch_send => 200 OK (96.308307ms)
    client.go:356: POST /_matrix/client/r0/join/!GkmAGvcDmllsuqLgZA:hs1 => 200 OK (808.747133ms)
    client.go:356: GET /_matrix/client/r0/rooms/!GkmAGvcDmllsuqLgZA:hs1/messages => 200 OK (83.415512ms)
```

After:
```
=== CONT  TestBackfillingHistory
    client.go:357: GET hs1/_matrix/client/r0/sync => 200 OK (29.885812ms)
    client.go:357: POST hs1/_matrix/client/unstable/org.matrix.msc2716/rooms/!jbwgZJKNOedwNWRdop:hs1/batch_send => 200 OK (96.173807ms)
    client.go:357: POST hs2/_matrix/client/r0/join/!jbwgZJKNOedwNWRdop:hs1 => 200 OK (808.849665ms)
    client.go:357: GET hs2/_matrix/client/r0/rooms/!jbwgZJKNOedwNWRdop:hs1/messages => 200 OK (73.667196ms)
```
@MadLittleMods MadLittleMods force-pushed the eric/msc2716-backfilling-history branch from 95cf005 to 425206d Compare July 14, 2021 00:13
@MadLittleMods
Copy link
Contributor Author

MadLittleMods commented Jul 14, 2021

@kegsay Can you give this another look?

I don't want to keep adding to it when we were so close to merge. If you only want to approve up to a certain point, I can create another branch with all of the updates and we can merge at any commit. (trying to avoid the conflicts with juggling many branches)

@kegsay
Copy link
Member

kegsay commented Jul 15, 2021

Yep sorry, I'll look at this again

tests/msc2716_test.go Outdated Show resolved Hide resolved
Copy link
Member

@kegsay kegsay left a comment

Choose a reason for hiding this comment

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

Let's merge this now.

@MadLittleMods MadLittleMods merged commit 91b0879 into master Jul 15, 2021
@MadLittleMods
Copy link
Contributor Author

Thanks for the review @kegsay 🦓 Lots of great improvements!

The functional client you refactored feels a lot better too!

MadLittleMods added a commit that referenced this pull request Jul 16, 2021
MadLittleMods added a commit that referenced this pull request Jul 16, 2021
MadLittleMods added a commit that referenced this pull request Jul 16, 2021
kegsay pushed a commit that referenced this pull request Jul 16, 2021
kegsay pushed a commit that referenced this pull request Jul 19, 2021
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