complement icon indicating copy to clipboard operation
complement copied to clipboard

Consider using a single file-naming scheme and test grouping methodology

Open ShadowJonathan opened this issue 4 years ago • 2 comments

Currently, while browsing the test repository, i find it difficult to navigate to tests i expect to exist, and/or not to exist, as the file naming scheme (and test grouping) seems to be all over the place.

I think it'll be worth it (for organisational purposes) to organise files and scope tests such that someone new coming into the repository can find their way and discover related tests better.


Some current complaints and notes are that some files seem to inherit sytest's vague naming scheme (apidoc, federation, etc.), some simply document and test MSCs, and some restrict themselves to very narrow scopes (account_change_password_pushers_test.go)

ShadowJonathan avatar Dec 01 '21 18:12 ShadowJonathan

Indeed, there is no consistent mandated way to name test files. There's a few constraints you need to consider:

  • Up until 12 days ago it was not possible to skip an individual test based on the homeserver, you had to skip an entire file using Go build tags. This is likely why there are some hyper-specific filenames as they are for certain failing tests.
  • Early in development I copied the sytest file naming scheme to more easily aid in test discovery: if you were comfortable with where tests were in sytest then you'd be comfortable in Complement too. This starts to fall apart when there are Complement-only tests.
  • MSCs in particular are optional which need to be opted in via build tags, so they have to be in their own file.
  • Directories are special due to the way the Complement test runner works (it just uses go test). Each directory is a package which is run concurrently by default by go test. Complement has some limitations (currently) around concurrent tests, in that you cannot current run multiple mock federation servers at once as it always binds to the same port: 8448 so running tests concurrently would get "port in use" errors. This is why there is a csapi directory which does not test federation at all, and then an "everything else" /tests directory. This isn't great but is something I am aware of.

kegsay avatar Dec 01 '21 18:12 kegsay

From https://github.com/matrix-org/complement/pull/289;

As of this PR, it will now (in theory) be possible to scale out tests indefinitely as we no longer have any central point/resource which all tests rely on. This should make it possible to split /tests into more packages which make sense from a Matrix PoV.

For now, i'm recommending a simple two-directory layering structure;

{client/fed} -> area (media/sync/room/alias, etc.)

ShadowJonathan avatar Jan 22 '22 15:01 ShadowJonathan