Skip to content

Run tests with pg_regress#50

Merged
Ngalstyan4 merged 3 commits intolanterndata:mainfrom
var77:feature/pg-regress
Aug 14, 2023
Merged

Run tests with pg_regress#50
Ngalstyan4 merged 3 commits intolanterndata:mainfrom
var77:feature/pg-regress

Conversation

@var77
Copy link
Copy Markdown
Collaborator

@var77 var77 commented Aug 12, 2023

Description

Run tests with pg_regress instead of doing it manually

TODO

  • Remove logs and comments from test results
  • Make tests work with filters

Issue

closes #43

@var77 var77 requested a review from Ngalstyan4 August 12, 2023 12:05
Copy link
Copy Markdown
Contributor

@Ngalstyan4 Ngalstyan4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@var77 var77 marked this pull request as ready for review August 13, 2023 13:39
@var77 var77 force-pushed the feature/pg-regress branch 2 times, most recently from 1666876 to 04afd97 Compare August 13, 2023 13:59
@var77 var77 force-pushed the feature/pg-regress branch from 04afd97 to 0503586 Compare August 13, 2023 14:10
@var77 var77 requested a review from Ngalstyan4 August 13, 2023 14:14
Copy link
Copy Markdown
Contributor

@Ngalstyan4 Ngalstyan4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@@ -1,13 +1,13 @@
#!/bin/bash
# bash strict mode
set -euo pipefail
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did this cause issues?
It forces the bash script to fail if some component fails and that is generally the sane thing to do.
See bash strict mode.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed it, so I can use trap command here

TEST_CASE_DB="ldb_${TESTFILE_NAME}"

# Drop db after each test on exit signal
function drop_db {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pg_regress has the option of running tests in parallel. How does that option interact with this dropping logic?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes currently all the tests are being run in parallel, and each test case is creating a database for it's own with the name like ldb_hnsw or ldb_hnsw_inserts like this.

# install lanterndb extension
psql "$@" -U postgres -d postgres -v ECHO=none -q -c "DROP DATABASE IF EXISTS ${TEST_CASE_DB};" 2>/dev/null
psql "$@" -U postgres -d postgres -v ECHO=none -q -c "DROP DATABASE IF EXISTS ${TEST_CASE_DB};" 2>/dev/null
psql "$@" -U postgres -d postgres -v ECHO=none -q -c "CREATE DATABASE ${TEST_CASE_DB};" 2>/dev/null
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From postgres docs

When using installcheck mode, these tests will create and destroy test databases whose names include regression, for example pl_regression or contrib_regression. Beware of using installcheck mode with an installation that has any non-test databases named that way.

It seems the upstream approach is to create and restroy databases hosting the tests, in stead of using an existing database. We should follow the same convention.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes actually we are creating and destroying the database after each test case, I just put IF EXISTS here, so if the script will be terminated in the middle it won't cause any issues in next run.

@Ngalstyan4 Ngalstyan4 merged commit d4bd24d into lanterndata:main Aug 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Move to pg_regress for regression testing

2 participants