Add ability to skip individual tests (#230)

* Add ability to skip individual tests

- Added `runtime.SkipIf`
- Re-combined federation test as it can now be individually skipped on Dendrite.

From a CI perspective, no changes are required as this uses the existing build
tags setup to detect HS impl.

* Update onboarding

Author: kegsay

ONBOARDING.md

@@ -189,7 +189,19 @@ Normally, server logs are only printed when one of the tests fail. To override t
 
 ### How do I skip a test?
 
-Use one of `t.Skipf(...)` or `t.SkipNow()`.
+To conditionally skip a *single* test based on the homeserver being run, add a single line at the start of the test:
+```go
+runtime.SkipIf(t, runtime.Dendrite)
+```
+To conditionally skip an entire *file* based on the homeserver being run, add a [build tag](https://pkg.go.dev/cmd/go#hdr-Build_constraints) at the top of the file which will skip execution of all the tests in this file if Complement is run with this flag:
+```go
+// +build !dendrite_blacklist
+```
+You can also do this based on features for MSC tests (which means you must run Complement *with* this tag for these tests *to run*):
+```go
+// +build msc_2836
+```
+See [GH Actions](https://github.com/matrix-org/complement/blob/master/.github/workflows/ci.yaml) for an example of how this is used for different homeservers in practice.
 
 ### Why do we use `t.Errorf` sometimes and `t.Fatalf` other times?
 

runtime/hs.go

@@ -0,0 +1,34 @@
+package runtime
+
+import "testing"
+
+const (
+	Dendrite = "dendrite"
+	Synapse  = "synapse"
+)
+
+var Homeserver string
+
+// Skip the test (via t.Skipf) if the homeserver being tested matches `hs`, else return.
+//
+// The homeserver being tested is detected via the presence of a `*_blacklist` tag e.g:
+//   go test -tags="dendrite_blacklist"
+// This means it is important to always specify this tag when running tests. Failure to do
+// so will result in a warning being printed to stdout, and the test will be run. When a new server
+// implementation is added, a respective `hs_$name.go` needs to be created in this directory. This
+// file pairs together the tag name with a string constant declared in this package
+// e.g. dendrite_blacklist == runtime.Dendrite
+func SkipIf(t *testing.T, hs string) {
+	t.Helper()
+	if Homeserver == hs {
+		t.Skipf("skipped on %s", hs)
+	}
+	if Homeserver == "" {
+		// they ran Complement without a blacklist so it's impossible to know what HS they are
+		// running, warn them.
+		t.Logf(
+			"WARNING: %s called runtime.SkipIf(%s) but Complement doesn't know which HS is running as it was run without a *_blacklist tag: executing test.",
+			t.Name(), hs,
+		)
+	}
+}

runtime/hs_dendrite.go

@@ -0,0 +1,7 @@
+// +build dendrite_blacklist
+
+package runtime
+
+func init() {
+	Homeserver = Dendrite
+}

runtime/hs_synapse.go

@@ -0,0 +1,7 @@
+// +build synapse_blacklist
+
+package runtime
+
+func init() {
+	Homeserver = Synapse
+}

tests/federation_room_join_test.go

@@ -20,6 +20,7 @@ import (
 	"github.com/matrix-org/complement/internal/federation"
 	"github.com/matrix-org/complement/internal/match"
 	"github.com/matrix-org/complement/internal/must"
+	"github.com/matrix-org/complement/runtime"
 )
 
 // This tests that joining a room with ?server_name= works correctly.
@@ -196,6 +197,60 @@ func TestJoinFederatedRoomWithUnverifiableEvents(t *testing.T) {
 		alice := deployment.Client(t, "hs1", "@alice:hs1")
 		alice.JoinRoom(t, roomAlias, nil)
 	})
+	t.Run("/send_join response with state with unverifiable auth events shouldn't block room join", func(t *testing.T) {
+		runtime.SkipIf(t, runtime.Dendrite) // https://github.com/matrix-org/dendrite/issues/2028
+		room := srv.MustMakeRoom(t, ver, federation.InitialRoomEvents(ver, charlie))
+		roomAlias := srv.MakeAliasMapping("UnverifiableAuthEvents", room.RoomID)
+
+		// create a normal event then modify the signatures
+		rawEvent := srv.MustCreateEvent(t, room, b.Event{
+			Sender:   charlie,
+			StateKey: &charlie,
+			Type:     "m.room.member",
+			Content: map[string]interface{}{
+				"membership": "join",
+				"name":       "This event has a bad signature",
+			},
+		}).JSON()
+		rawSig, err := json.Marshal(map[string]interface{}{
+			docker.HostnameRunningComplement: map[string]string{
+				string(srv.KeyID): "/3z+pJjiJXWhwfqIEzmNksvBHCoXTktK/y0rRuWJXw6i1+ygRG/suDCKhFuuz6gPapRmEMPVILi2mJqHHXPKAg",
+			},
+		})
+		must.NotError(t, "failed to marshal bad signature block", err)
+		rawEvent, err = sjson.SetRawBytes(rawEvent, "signatures", rawSig)
+		must.NotError(t, "failed to modify signatures key from event", err)
+		badlySignedEvent, err := gomatrixserverlib.NewEventFromTrustedJSON(rawEvent, false, ver)
+		must.NotError(t, "failed to make Event from badly signed event JSON", err)
+		room.AddEvent(badlySignedEvent)
+		t.Logf("Created badly signed auth event %s", badlySignedEvent.EventID())
+
+		// and now add another event which will use it as an auth event.
+		goodEvent := srv.MustCreateEvent(t, room, b.Event{
+			Sender:   charlie,
+			StateKey: &charlie,
+			Type:     "m.room.member",
+			Content: map[string]interface{}{
+				"membership": "leave",
+			},
+		})
+		// double-check that the bad event is in its auth events
+		containsEvent := false
+		for _, authEventID := range goodEvent.AuthEventIDs() {
+			if authEventID == badlySignedEvent.EventID() {
+				containsEvent = true
+				break
+			}
+		}
+		if !containsEvent {
+			t.Fatalf("Bad event didn't appear in auth events of state event")
+		}
+		room.AddEvent(goodEvent)
+		t.Logf("Created state event %s", goodEvent.EventID())
+
+		alice := deployment.Client(t, "hs1", "@alice:hs1")
+		alice.JoinRoom(t, roomAlias, nil)
+	})
 }
 
 // This test checks that users cannot circumvent the auth checks via send_join.