Allow "user-defined" list of skipped tests

[ShadowJonathan]

Currently, through the use of runtime.SkipIf, tests (from within that test) are skipped depending on which known runtime/server it is testing against.

This is fine considering that synapse and dendrite are closely developed with complement, however, there's a problem if server X comes into play.

Server X's developers are not part of Element or Matrix.org, and developed their own homeserver off on the side, engaging in the community and open spirit of matrix itself.

However, they are still in the middle of development on that homeserver, many features and endpoints not finalized, yet they still want to use Complement.

The current option would be to PR against Complement's codebase, and register their own server.

Then, every time they have added a feature, or complement adds another incompatible test, they would have to STW and wait until the skipIf addition or removal PR has proceeded.

Even still, if they would CI against :latest, and Complement adds new tests, old CI runs cannot be comprehensively retried unless the exact image hash is known and still up.

---

These points may be doable, however, they have a few breaking points;

• What if the developers (for whatever reason) refuse to use github, or simply do not find it feasable to contribute in the public?
• What if their homeserver project is semi-secretive, and it's name shouldn't be known?
• What about their development speed, if it has to constantly await complement's approval?

All of these issues could sap or otherwise burn those developer's motivation or patience to work on matrix, and it would raise the threshold of motivation and effort to develop matrix homeservers, while i think these issues are completely avoidable.

Synapse and Dendrite don't have to deal with some of these questions, because their development is pretty closely tied with Complement's developers and internal development channels. But, i hope that some of the above can illustrate the difficulty (and defacto "centralisation") that the current setup requires for a homeserver to start and keep using complement.

In this scenario, the homeserver awaits complement's development.

---

Let me propose the following instead;

Complement should try to read a complement.yml in the current directory when it starts to run, this file can give a blacklist or whitelist of test items that this run wishes to (exclusively) include or exclude.

(This list can also be compressed in JSON in an environment variable, such as COMPLEMENT_TASKLIST)

Complement then selects or unselects to-be-ran tests, this could possibly be done by a boilerplate checkSkip() invocation at the start of every test.

In this scenario, the homeserver developers could submit whitelist additions/blacklist removals to incrementally support more and more tests for their homeserver, with the definition file for such existing on their development branch, while complement only has to read that at runtime. Complement awaits the homeserver's development.

---

Additionally, some more features;

Tags could be introduced, something akin to tag:v1.1, which enables every test which is geared towards matrix v1.1 compatibility.

Globs for test names could be used, blacklist: ["test:federation*"], which would exclude all tests starting with "federation" (though, test naming depends a bit on #241 resolution, so this is likely not accurate, nor possibly useful)

[kegsay]

Many of your points can be answered by one of:

• forking Complement.
• Parsing the test output differently so test failures do not mean failure.

I like the idea of supporting custom homeservers in a flexible enough way that they don't need to send PRs to Complement all the time. However, which tests need to be run still need to be specified somehow. The easiest way to support this initially was to use build tags to include or exclude files, which was recently expanded to allow per-test granularity. At the time I implemented per-test skips, I mentioned in #complement:

it's unfortunate that Complement doesn't actually know the HS impl it is running (it just sees docker images) hence having to indirectly guess the impl via *_blacklist tags, but at least there's appropriate warnings if they are missing and a skipped test is found. I considered doing a feature matrix for it so we don't need to care about the HS impl (akin to browser feature detection vs user agent sniffing) but it becomes much more complicated and difficult to understand and introduces a maintenance burden to keep the feature matrix up to date. It's easier if Complement master just tracks $hs master and the presence/absence of tags indicates supported features as it gives us the most flexibility and remains very easy to understand, at the cost of having to trawl through tests to remove the tag or SkipIf function, but that seems a fair trade-off.

Using a test-name-per-line file style approach would help here, at the cost of making test names an intrinsic part of Complement's API surface -- you cannot just reshuffle tests or fix a typo in a test name without breaking people. In addition, the skipping inversion makes it hard to discover new tests - you cannot just git pull up to master and see a bunch of new failing tests, you have to comb through the diff to see the test names added and then add them in on your end, not particularly fun.

Unfortunately tags don't help resolve this either as they then end up in the same boat we have today: all the tagged tests will be run, and if 90% of your tests with that tag pass, it's a shame to not run them just for the remaining 10% (which is a concrete problem we had recently when Dendrite implemented restricted rooms but doesn't yet know about spaces).

All options suck in their own way, but the current approach does not prevent HS developers from implementing a subset of Complement tests if they just parse the test output and modify the exit code based on a whitelist of test names that should pass (very similar to how Dendrite has EXPECTED FAIL on sytest). This allows a whitelist approach for other HSes, at the cost of maintaining an up-to-date list of test names. This is much more preferable from my side as I do not want to make the test names a fundamental part of the API surface of Complement: if you want to parse the output and filter on test names then be my guest, but we make no guarantees for stability.

[ShadowJonathan]

Alright, I understand, thanks for the comprehensive response.

Unfortunately tags don't help resolve this either as they then end up in the same boat we have today: all the tagged tests will be run, and if 90% of your tests with that tag pass, it's a shame to not run them just for the remaining 10%

I had this in mind, but i didn't include it in the initial bit: denys (from a blacklist) are prioritised over an allow (from a whitelist), so specifying

whitelist:
  - tag:v1.1

blacklist:
  - test1
  - test2
  - test3
  - test4

Would indeed exclude that 10%, until those blacklist entries can be removed one by one by those developers.

---

Using a test-name-per-line file style approach would help here, at the cost of making test names an intrinsic part of Complement's API surface -- you cannot just reshuffle tests or fix a typo in a test name without breaking people.

I think that this can be helped by versioning complement releases, and listing breaking namechanges as a part of it. You could still change names, but then you can also warn people about it.

Another venue would be to deprecate the old name, but emit warnings when it is used to skip, up until a particular release where that'd be removed.

---

In addition, the skipping inversion makes it hard to discover new tests - you cannot just git pull up to master and see a bunch of new failing tests, you have to comb through the diff to see the test names added and then add them in on your end, not particularly fun.

I think this could be resolved by adding a command which searches for excluded tests (which are not matched in either blacklist or whitelist), and (maybe) display them in order of when they got added (though for the last one, it would require more metadata embedded into the tests, which i dont think is particularly wanted)

[kegsay]

This isn't something that we're going to do in an official capacity. If you want this, feel free to do it sytest-style and filter on test names, but the maintenance burden will be with you then.

[sumnerevans]

I find that "just fork complement" is a very unfortunate solution for homeserver developers who want to use this tool as a black-box test suite for their homeserver. However, maybe this is just not a use-case that is meant to be supported.

What we currently do at Beeper is:

1. Fork Complement and add Hungryserv to the list of homeservers beeper@3728727
2. Manually blacklist certain sub tests
3. Whitelist root tests using a file containing in our Hungryserv repository containing the names of the tests and a script that joins them together in a regex
4. Add a few of our own tests to the fork

The first and second items could be solved by implementing blacklisting functionality, but there is still no way to point Complement at an additional set of tests for homeserver-specific functionality.

Ideally, we would have a way to blacklist tests (and add homeserver-specific tests, but that's a different issue) without forking. I understand the aversion to adding strings to the API boundary, but it seems better than having a fork for every homeserver implementation, as forks will drift if not regularly rebased.

[kegsay]

there is still no way to point Complement at an additional set of tests for homeserver-specific functionality.

This is basically #226 which we do plan on supporting, at some point.