Do once-over on all SkipIfs in codebase, adding issue numbers, un-skipping passing tests
#519
Conversation
| ) | ||
|
|
||
| func TestRelations(t *testing.T) { | ||
| runtime.SkipIf(t, runtime.Dendrite) // not supported |
There was a problem hiding this comment.
I did not spot that, should I remove these changes so they don't conflict?
There was a problem hiding this comment.
I guess leave it, they'll conflict and they can fix it there.
| // sytest: Rejects invalid device keys | ||
| t.Run("Rejects invalid device keys", func(t *testing.T) { | ||
| runtime.SkipIf(t, runtime.Dendrite, runtime.Synapse) // Dendrite doesn't pass, Synapse has it blacklisted | ||
| runtime.SkipIf(t, runtime.Dendrite, runtime.Synapse) // Blacklisted on Synapse, Dendrite FIXME: https://github.com/matrix-org/dendrite/issues/2804 |
There was a problem hiding this comment.
Is there a corresponding Synapse issue?
There was a problem hiding this comment.
There is one in this repo: #517
I looked for one on synapse, but didn't open one there as I wasn't entirely sure what the reasoning was why these tests were blacklisted in the first place, so I opened one here to start that inquiry.
| // sytest: Existing members see new members' presence | ||
| t.Run("Existing members see new members' presence", func(t *testing.T) { | ||
| runtime.SkipIf(t, runtime.Dendrite) // Still failing | ||
| runtime.SkipIf(t, runtime.Dendrite) // FIXME: https://github.com/matrix-org/dendrite/issues/2803 |
There was a problem hiding this comment.
#516 removes the line entirely, the PR might conflict
The reason it can't be done here is because that PR also introduces changes to fix the test itself
2 participants
Seeing as almost all SkipIfs in complement are due to the fact the originating homeservers do not implement the functionality, I did a once-over to check all of them, and paired them with those issues, and standardized the format of
SkipIfcomments along the way.Like this, the intended way how to skip a test on a homeserver then always comes paired with an issue to watch or resolve to remove that skip, else these skips would bitrot and the related tests aren't executed, potentially hiding non-compliant behaviour.
Furthermore, some tests did pass inbetween then and now, those skips have been removed.