Skip to content

Could Complement be extended to load and run out-of-repository tests? #226

Description

@reivilibre

The context here is that we 'need' some way to run integration tests for non-Matrix-standard features, such as those implemented by custom Synapse modules or maybe even Synapse-specific behaviours (if/where there is good reason).

Without knowing too much, Complement seems basically perfect for this use case, but it really wouldn't make sense to add these kinds of tests straight to Complement's repository.

It would be nice to have out-of-repository test suites (for example: alongside a Synapse module, say https://github.com/matrix-org/synapse-user-restrictions/, but you can also imagine weirder customer-specific modules).
Is Complement suitable for being extended this way? What would it take?

I'm not familiar with Go, so I can't easily suggest the right approach (I can speculate, but I'll refrain from doing so for now).

Activity

  1. kegsay commented on Nov 19, 2021

    @kegsay
    Member

    Yes, it is possible but Complement isn't quite yet there for out-of-repo tests yet because it lacks a public API. To elaborate..

    Complement just runs some Go tests so it's possible to set up the test framework trivially: it's just go test ./your-directory. Complement tests call functions/structs that are inside /internal and that is a problem. "internal" is a special name in Go and it means imports only work if the package doing the importing is on the same path as the imported package, which won't be the case for you if you wanted to do out of repo tests.

    If the directory /internal is renamed then it should be possible to just import packages from Complement and have it work, provided you have the bootstrapping code from main_test.go e.g https://github.com/matrix-org/complement/blob/master/tests/main_test.go#L30

    Now that's an easy fix, but there's a reason why it's internal: the API surface isn't stable nor do we (in practice I) make any guarantees that it will remain so.

    I can work on opening up the API to allow this use case though as it definitely seems desirable and useful. A brief checklist for this to be done:

    • All tests in /tests should not be referring to any imports in /internal e.g "github.com/matrix-org/complement/internal/b"
    • The b,must and match packages have already been used heavily and are geared for public consumption so they can just be shifted out.
    • The remaining packages need to be audited to see how tests in practice use them. Back when the framework was set up we had very few tests, which isn't the case now. There will likely be common usage patterns which we should support, and hide implementation details much more to allow the inner workings of Complement to change without breaking all the tests.

    I can start working on this over the next few days (I need to make some more quality of life improvements to Complement anyway around blacklists) but if you need a quick fix then you can just fork and rename internal.

  2. self-assigned this
    on Nov 19, 2021
  3. reivilibre commented on Nov 19, 2021

    @reivilibre
    ContributorAuthor

    Thanks for your detailed response!
    This isn't urgent (as far as I'm aware), so not trying to rush anything, but it's a question that's been floating around for some time and last time I thought of asking you, you were on hol. (Thinking about it, it made more sense to be written down as an issue anyway, so I posted here instead.)

    What you're suggesting makes sense, though, so thanks :).

  4. kegsay commented on Jan 6, 2022

    @kegsay
    Member

    I haven't forgotten about this.

  5. kegsay commented on Feb 7, 2022

    @kegsay
    Member

    Still haven't forgotten about this - fixing CAs and allowing more concurrency has taken priority.

  6. kegsay commented on Mar 28, 2022

    @kegsay
    Member

    This remains a longer term goal, but requires complement's API to be stabilised.

  7. kegsay commented on Jul 25, 2022

    @kegsay
    Member

    I would very much like this for the sliding sync proxy, as having a coherent Go API is useful...

  8. ShadowJonathan commented on Nov 14, 2022

    @ShadowJonathan
    Contributor

    We can also choose to not commit to a stable interface (yet), and allow consumers (such as synapse) to still use the utilities at disposal, but warn about the instability.

    Though this means that synapse's out-of-tree tests would have to be compiled and ran independently against a frozen version of complement (via go.sum or other).

    Personally, for the sake of simplicity, this would be much less a bother than committing to a stable interface now, and would allow to convert the remaining amount of sytests (that test synapse-exclusive behaviour) to complement-using tests, while we still iterate on the design of the API.

  9. added this to the v1 milestone on Oct 2, 2023
  10. kegsay commented on Oct 2, 2023

    @kegsay
    Member

    Tagged for v1 specifically to address:

    • Making b public.
    • Making must public.
    • Making match public.
    • Exposing the general Deploy machinery.
  11. kegsay commented on Oct 18, 2023

    @kegsay
    Member

    This can now be done for CSAPI tests only. Federation tests need internal/federation exported, and possibly internal/web.

  12. kegsay commented on Nov 27, 2023

    @kegsay
    Member

    Federation tests can now be run out-of-repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

enhancementNew feature or request

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions