Skip to content

Update thumbnail tests to include _matrix/client/v1/media/thumbnail endpoint #728

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jul 10, 2024
Merged

Update thumbnail tests to include _matrix/client/v1/media/thumbnail endpoint #728

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1089070
update thumbnail tests to include _matrix/client/v1/media endpoint
H-Shay Jul 2, 2024
0c5fb7d
add subtests for new endpoints
H-Shay Jul 8, 2024
edca237
skip dendrite for now
H-Shay Jul 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 32 additions & 4 deletions tests/media_thumbnail_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@ package tests

import (
"bytes"
"github.com/matrix-org/complement/runtime"
"image/jpeg"
"image/png"
"io"
"net/http"
"net/url"
"strings"
"testing"
Expand All @@ -29,7 +31,12 @@ func TestLocalPngThumbnail(t *testing.T) {

uri := alice.UploadContent(t, data.LargePng, fileName, contentType)

fetchAndValidateThumbnail(t, alice, uri)
fetchAndValidateThumbnail(t, alice, uri, false)
Copy link
Contributor

Choose a reason for hiding this comment

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

There's nothing wrong with this, but for this and the Remote test, I think it'd be better if the old and new endpoints were separate tests.

Supposing you had a defect in one of the versions of endpoints, you'd want the test results to show which version was working fine and which version wasn't.

Equally, if a new cutting-edge homeserver Syndruit wants to skip implementing the legacy endpoints, they may want to blacklist the tests just for the old version.

I asked in the Complement room and it seems doing this as subtests would work for this and it looks fairly easy to do: seems like you just wrap each part of the test like this:

Suggested change
fetchAndValidateThumbnail(t, alice, uri, false)
t.Run("unauthenticated", func(t *testing.T) {
fetchAndValidateThumbnail(t, alice, uri, false)
})

& similar for the other ones (The string argument to t.Run is a subtest name)

t.Run("test /_matrix/client/v1/media endpoint", func(t *testing.T) {
runtime.SkipIf(t, runtime.Dendrite)
fetchAndValidateThumbnail(t, alice, uri, true)
})

}

// sytest: Remote media can be thumbnailed
Expand All @@ -45,15 +52,36 @@ func TestRemotePngThumbnail(t *testing.T) {

uri := alice.UploadContent(t, data.LargePng, fileName, contentType)

fetchAndValidateThumbnail(t, bob, uri)
fetchAndValidateThumbnail(t, bob, uri, false)

t.Run("test /_matrix/client/v1/media endpoint", func(t *testing.T) {
runtime.SkipIf(t, runtime.Dendrite)
fetchAndValidateThumbnail(t, bob, uri, true)
})

// Remove the AccessToken and try again, this should now return a 401.
alice.AccessToken = ""
origin, mediaId := client.SplitMxc(uri)
res := alice.Do(t, "GET", []string{"_matrix", "client", "v1", "media", "thumbnail", origin, mediaId})
if res.StatusCode != http.StatusUnauthorized {
t.Fatalf("expected HTTP status: %d, got %d", http.StatusUnauthorized, res.StatusCode)
}
}

func fetchAndValidateThumbnail(t *testing.T, c *client.CSAPI, mxcUri string) {
func fetchAndValidateThumbnail(t *testing.T, c *client.CSAPI, mxcUri string, authenticated bool) {
t.Helper()

origin, mediaId := client.SplitMxc(mxcUri)

res := c.MustDo(t, "GET", []string{"_matrix", "media", "v3", "thumbnail", origin, mediaId}, client.WithQueries(url.Values{
var path []string

if authenticated {
path = []string{"_matrix", "client", "v1", "media", "thumbnail", origin, mediaId}
} else {
path = []string{"_matrix", "media", "v3", "thumbnail", origin, mediaId}
}

res := c.MustDo(t, "GET", path, client.WithQueries(url.Values{
"width": []string{"32"},
"height": []string{"32"},
"method": []string{"scale"},
Expand Down
Loading