From b8c7a9844c2470d7a468dcb1963ec25dfcb5995f Mon Sep 17 00:00:00 2001 From: Naisila Puka <37271756+naisila@users.noreply.github.com> Date: Thu, 3 Nov 2022 10:55:50 +0300 Subject: [PATCH] Add docs on handling alternative test outputs (#6469) I recently cleaned up our test suite from redundant test outputs: #6111 #6140 #6214 #6140 #6434 I had to check many files manually, as they didn't have any documentation on why the alternative test output existed in the first place. Adding a section in our test docs to remind developers to add alternative test outputs with enough information/keywords. --- src/test/regress/README.md | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/test/regress/README.md b/src/test/regress/README.md index 25ee88ecd..b2a6c77d3 100644 --- a/src/test/regress/README.md +++ b/src/test/regress/README.md @@ -100,6 +100,38 @@ To automatically setup a citus cluster in tests we use our then starts the standard postgres test tooling. You almost never have to change this file. +## Handling different test outputs + +Sometimes the test output changes because we run tests in different configurations. +The most common example is an output that changes in different Postgres versions. +We highly encourage to find a way to avoid these test outputs. +You can try the following, if applicable to the changing output: +- Change the test such that you still test what you want, but you avoid the different test outputs. +- Reduce the test verbosity via: `\set VERBOSITY terse`, `SET client_min_messages TO error`, etc +- Drop the specific test lines altogether, if the test is not critical. +- Use utility functions that modify the output to your preference, +like [coordinator_plan](https://github.com/citusdata/citus/blob/main/src/test/regress/sql/multi_test_helpers.sql#L23), +which modifies EXPLAIN output +- Add [a normalization rule](https://github.com/citusdata/citus/blob/main/ci/README.md#normalize_expectedsh) + +Alternative test output files are highly discouraged, so only add one when strictly necessary. +In order to maintain a clean test suite, make sure to explain why it has an alternative +output in the test header, and when we can drop the alternative output file in the future. + +For example: + +```sql +-- +-- MULTI_INSERT_SELECT +-- +-- This test file has an alternative output because of the change in the +-- display of SQL-standard function's arguments in INSERT/SELECT in PG15. +-- The alternative output can be deleted when we drop support for PG14 +-- +``` +Including important keywords, like "PG14", "PG15", "alternative output" will +help cleaning up in the future. + ## Randomly failing tests In CI sometimes a test fails randomly, we call these tests "flaky". To fix these