Skip to content

Commit

Permalink
Convert columnar tap tests to pytest (citusdata#6720)
Browse files Browse the repository at this point in the history
Having as little Perl as possible in our repo seems a worthy goal. Sadly
Postgres its Perl based TAP infrastructure was the only way in which we
could
run tests that were hard to do using only SQL commands. This change adds
infrastructure to run such "application style tests" using python and
converts all our existing Perl TAP tests to this new infrastructure.

Some of the helper functions that are added in this PR are currently
unused. Most of these will be used by the CDC PR that depends on this.
Some others are there because they were needed by the PgBouncer test
framework that this is based on, and the functions seemed useful enough
to citus testing to keep.

The main features of the test suite are:
1. Application style tests using a programming language that our
developers know how to write.
2. Caching of Citus clusters in-between tests using the ["fixture"
pattern][fixture] from `pytest` to achieve speedy tests. To make this
work in practice any changes made during a test are automatically
undone. Schemas, replication slots, subscriptions, publications are
dropped at the end of each test. And any changes made by `ALTER SYSTEM`
or manually editing of `pg_hba.conf` are undone too.
3. Automatic parallel execution of tests using the `-n auto` flag that's
added by `pytest-xdist`. This improved the speed of tests greatly with
the similar test framework I created for PgBouncer. Right now it doesn't
help much yet though, since this PR only adds two tests (one of which
takes ~10 times longer than the other).

Possible future improvements are:
1. Clean up even more things at the end of each test (e.g. users that
were created). These are fairly easy to add, but I have not done so yet
since they were not needed yet for this PR or the CDC PR. So I would not
be able to test the cleanup easily.
2. Support for query block detection similar to what we can now do using
isolation tests.

[fixture]: https://docs.pytest.org/en/6.2.x/fixture.html
  • Loading branch information
JelteF authored Mar 31, 2023
1 parent 01ea5f5 commit 7b60cdd
Show file tree
Hide file tree
Showing 22 changed files with 1,379 additions and 465 deletions.
77 changes: 47 additions & 30 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ orbs:
parameters:
image_suffix:
type: string
default: '-v4b2ae97'
default: '-v087ecd7'
pg13_version:
type: string
default: '13.10'
Expand Down Expand Up @@ -269,6 +269,41 @@ jobs:
- coverage:
flags: 'test_<< parameters.old_pg_major >>_<< parameters.new_pg_major >>,upgrade'

test-pytest:
description: Runs pytest based tests
parameters:
pg_major:
description: 'postgres major version'
type: integer
image:
description: 'docker image to use as for the tests'
type: string
default: citus/failtester
image_tag:
description: 'docker image tag to use'
type: string
docker:
- image: '<< parameters.image >>:<< parameters.image_tag >><< pipeline.parameters.image_suffix >>'
working_directory: /home/circleci/project
steps:
- checkout
- attach_workspace:
at: .
- install_extension:
pg_major: << parameters.pg_major >>
- configure
- enable_core
- run:
name: 'Run pytest'
command: |
gosu circleci \
make -C src/test/regress check-pytest
no_output_timeout: 2m
- stack_trace
- coverage:
flags: 'test_<< parameters.pg_major >>,pytest'


test-arbitrary-configs:
description: Runs tests on arbitrary configs
parallelism: 6
Expand Down Expand Up @@ -558,7 +593,7 @@ jobs:
testForDebugging="<< parameters.test >>"
if [ -z "$testForDebugging" ]; then
detected_changes=$(git diff origin/main... --name-only --diff-filter=AM | (grep 'src/test/regress/sql/.*.sql\|src/test/regress/spec/.*.spec' || true))
detected_changes=$(git diff origin/main... --name-only --diff-filter=AM | (grep 'src/test/regress/sql/.*\.sql\|src/test/regress/spec/.*\.spec\|src/test/regress/citus_tests/test/test_.*\.py' || true))
tests=${detected_changes}
else
tests=$testForDebugging;
Expand Down Expand Up @@ -861,42 +896,30 @@ workflows:
image: citus/failtester
make: check-failure

- tap-test-citus: &tap-test-citus-13
name: 'test-13_tap-recovery'
suite: recovery
- test-pytest:
name: 'test-13_pytest'
pg_major: 13
image_tag: '<< pipeline.parameters.pg13_version >>'
requires: [build-13]
- tap-test-citus:
<<: *tap-test-citus-13
name: 'test-13_tap-columnar-freezing'
suite: columnar_freezing

- tap-test-citus: &tap-test-citus-14
name: 'test-14_tap-recovery'
suite: recovery
- test-pytest:
name: 'test-14_pytest'
pg_major: 14
image_tag: '<< pipeline.parameters.pg14_version >>'
requires: [build-14]
- tap-test-citus:
<<: *tap-test-citus-14
name: 'test-14_tap-columnar-freezing'
suite: columnar_freezing

- tap-test-citus: &tap-test-citus-15
name: 'test-15_tap-recovery'
suite: recovery
- test-pytest:
name: 'test-15_pytest'
pg_major: 15
image_tag: '<< pipeline.parameters.pg15_version >>'
requires: [build-15]

- tap-test-citus:
<<: *tap-test-citus-15
name: 'test-15_tap-columnar-freezing'
suite: columnar_freezing
- tap-test-citus:
<<: *tap-test-citus-15
name: 'test-15_tap-cdc'
suite: cdc
pg_major: 15
image_tag: '<< pipeline.parameters.pg15_version >>'
requires: [build-15]

- test-arbitrary-configs:
name: 'test-13_check-arbitrary-configs'
Expand Down Expand Up @@ -947,8 +970,6 @@ workflows:
- test-13_check-follower-cluster
- test-13_check-columnar
- test-13_check-columnar-isolation
- test-13_tap-recovery
- test-13_tap-columnar-freezing
- test-13_check-failure
- test-13_check-enterprise
- test-13_check-enterprise-isolation
Expand All @@ -967,8 +988,6 @@ workflows:
- test-14_check-follower-cluster
- test-14_check-columnar
- test-14_check-columnar-isolation
- test-14_tap-recovery
- test-14_tap-columnar-freezing
- test-14_check-failure
- test-14_check-enterprise
- test-14_check-enterprise-isolation
Expand All @@ -987,8 +1006,6 @@ workflows:
- test-15_check-follower-cluster
- test-15_check-columnar
- test-15_check-columnar-isolation
- test-15_tap-recovery
- test-15_tap-columnar-freezing
- test-15_check-failure
- test-15_check-enterprise
- test-15_check-enterprise-isolation
Expand Down
2 changes: 1 addition & 1 deletion .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ trim_trailing_whitespace = true
insert_final_newline = unset
trim_trailing_whitespace = unset

[*.{sql,sh,py}]
[*.{sql,sh,py,toml}]
indent_style = space
indent_size = 4
tab_width = 4
Expand Down
32 changes: 32 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,35 @@ profile = 'black'

[tool.black]
include = '(src/test/regress/bin/diff-filter|\.pyi?|\.ipynb)$'

[tool.pytest.ini_options]
addopts = [
"--import-mode=importlib",
"--showlocals",
"--tb=short",
]
pythonpath = 'src/test/regress/citus_tests'
asyncio_mode = 'auto'

# Make test discovery quicker from the root dir of the repo
testpaths = ['src/test/regress/citus_tests/test']

# Make test discovery quicker from other directories than root directory
norecursedirs = [
'*.egg',
'.*',
'build',
'venv',
'ci',
'vendor',
'backend',
'bin',
'include',
'tmp_*',
'results',
'expected',
'sql',
'spec',
'data',
'__pycache__',
]
56 changes: 0 additions & 56 deletions src/test/columnar_freezing/Makefile

This file was deleted.

7 changes: 0 additions & 7 deletions src/test/columnar_freezing/postgresql.conf

This file was deleted.

52 changes: 0 additions & 52 deletions src/test/columnar_freezing/t/001_columnar_freezing.pl

This file was deleted.

This file was deleted.

2 changes: 0 additions & 2 deletions src/test/recovery/.gitignore

This file was deleted.

Loading

0 comments on commit 7b60cdd

Please sign in to comment.