Suggestion for a new problem + solution:

🏗️ consistent setup between CI and local

Different parts of the test suite rely on different assumptions: that there's a version of graphql-engine running, that it was started with some specific environment variables, that the proper environment variables are set... And there isn't one consistent way of running the tests; the CI uses a verbose run-tests.sh, which runs all or parts of the test suite with different configurations, while locally, for devs, dev.sh test --integration is the preferred way. This leads to a lot of complexity and errors:

• some tests are always skipped locally
• new tests can work on CI but break local workflows
• the CI is probably testing more than it needs by running the suite in each configuration
• the metadata database running separately from the tests means that it is persisted from one group of tests to the next, and that previous test failures can impact other unrelated tests down the line

goal: running the tests should be done the same way on CI and locally, and the test suite itself should be responsible for its own setup

suggested improvements:

• invert the dependency: it's the test suite that, for each feature, decides how the engine should be started, with which environment flags

Thanks for putting this together Abby!

• figure out, or document, how to easily identify relevant tests for a particular feature, such that devs don't have to run the entire suite.

We could in theory hack something together with haskell code coverage, for each integration test class/module store a code coverage report, then (waves hands) combine that with the local git diff...

adding test support for a new backend (e.g. SQL Server, BigQuery) is currently a 4 or 5-step process involving a lot of code and yaml-file duplication, with many minor, manual and error-prone tweaks (see linked examples). This is likely to discourage extensive testing and community contribution.

The process linked for adding a new backend doesn't sound too bad to me, but I probably have low expectations... but I definitely agree with the goal of encouraging thorough testing against new backends.

our pytest output has a pretty low signal:noise ratio, and it's often difficult to understand the cause of a test failure, if there is one. Example: a 8600-line log for what turned out to be a flaky test that passed on a second run.

One thing I've noticed is some sets of tests within a class are stateful, so if an earlier test fails the later ones will too. The appropriate thing is I think to just make them one big test (commenting the different phases).

identifying which tests are actually run and which setup/teardown files support them seems to be a manual lookup of test files to string filepaths. Example. This contributes to cargo-culting redundant files and makes refactoring/unused file cleanup difficult.

Ya it's annoying to match setup/teardown specs to tests... I think an improvement would be to have every test class have its own set of setup/teardown files, following some obvious naming convention, even if that meant duplicating some of these files

Thanks so much for these suggestions @jberryman, I've included them in the description above.

The process linked for adding a new backend doesn't sound too bad to me

I hope Evie doesn't mind me linking to her PR as an example to illustrate that the process in practice turns out to be much more clunky.

The request format for v1/query (existing Postgres tests) and v2/query (new backend tests) differs ever so slightly, and new backend tests require split setup files for v1/metadata and v2/query requests, which introduces a lot of almost entirely duplicated files, with minor manual alterations.

Combined with unclear test logs, testing new backends in bulk can be difficult due to errors or test failures that might be caused by SQL syntax, .yaml file format, missing setup, stateful tests or any combination thereof. In the linked example, this was prohibitive enough to delay testing of an entire feature set (functions & mutations).

For "reasoning about test state and coverage":

goal: unsure, to be honest. Is it enough to find and remove dead code regularly, so that we have more confidence that the existing files are relevant? In which case...

How about unifying the way we do setup/teardown such that there is a single way to do so? The syntax might become slightly more complex, but the advantage is that it'll always be clear how to do it, and there's no more redundancies.

Additionally, I think adding new tests should only rely on creating new files. It's really easy to forget to update the Python classes to add the appropriate method and get PRs accepted.

I've created separate issues for each of the proposed goals listed above; we'll aim to prioritise these in the server team sprints in the next few weeks, but please consider these individual issues open to feedback, discussion and contribution.

Otherwise closing this RFC with the plan to keep the individual issues updated.