Conversation
|
This generally looks fine to me. I'm a little wary of adding yet another way of excluding tests (on top of the test whitelist/blacklist), and generally the right solution to me sounds like we should just rewrite these tests in Synapse's unit test suite. Then again, I don't particularly want to do the latter... so this seems like a good alternative. Though I'm not entirely sure if it could be useful in any other case. @richvdh curious for your thoughts, but otherwise lgtm. |
run-tests.pl
Outdated
| my @tokens = split /::/, $SERVER_IMPL; | ||
| if ( $tokens[0] ne $params{implementation_specific} ) { |
There was a problem hiding this comment.
This feels slightly too magical and fragile; could we give the Homeserver classes an implementation_name method (see server_name as an existing example) or something instead?
There was a problem hiding this comment.
I have added the implementation_name method to the HomeserverFactory, since that seems to be easily accessible from run_tests.pl.
richvdh
left a comment
There was a problem hiding this comment.
can you add a stub impl to HomeserverFactory itself, with some comments to document the interface?
(ideally HomeserverFactory would have better documentation in general)
| sub implementation_name | ||
| { | ||
| return "synapse"; | ||
| } | ||
|
|
There was a problem hiding this comment.
this should be unnecessary, as it will be inherited from SyTest::HomeserverFactory::Synapse ?
| sub implementation_name | ||
| { | ||
| return "dendrite"; | ||
| } |
This PR adds a test parameter
implementation_specific, which instructs Sytest to run a test for a specific implementation only. It also marks the admin tests as Synapse-specific.