Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support integration tests #43

Closed
slifty opened this issue Jul 12, 2022 · 12 comments · Fixed by #54
Closed

Support integration tests #43

slifty opened this issue Jul 12, 2022 · 12 comments · Fixed by #54
Assignees

Comments

@slifty
Copy link
Member

slifty commented Jul 12, 2022

We're about to need to run integration tests (I imagine many of our tests will rely on DB access in fact!)

This will mean three test commands:

npm run test:unit -- runs unit tests (no database)
npm run test:integration -- runs untegration tests
npm run test -- runs both sets of tests

Part of this issue will be figuring out the right way to do integration tests so that they're run in parallel but don't step on each other.

Potential resources I've found:

https://dev.to/walrusai/testing-database-interactions-with-jest-519n
https://jestjs.io/docs/setup-teardown
https://jestjs.io/docs/configuration#testmatch-arraystring

@bickelj
Copy link
Contributor

bickelj commented Jul 12, 2022

Is it possible to specify different postgres schemas to create and use when testing rather than the default public schema? I don't think any of our migrations or code will refer to a schema. By schema here I mean the postgres construct that lies between database and table.

@slifty
Copy link
Member Author

slifty commented Jul 12, 2022

@bickelj not sure what you mean exactly.

it's possible to mock databases, which I think would be useful in unit tests, but I think integration tests would be more robust using the true pg database + migrations.

(Is any of that useful / responsive to your question?)

@bickelj
Copy link
Contributor

bickelj commented Jul 12, 2022

@slifty I see that some of the above resources mention creating separate databases for test isolation, which makes sense, like create database blah owner blah but there is also the possibility of re-using the same database with separate schemas, such as connect to database blah, but then create schema test2, etc. I don't recall if create schema is lighter weight or about the same. If it's about the same as create database then my suggestion is moot. By default, the schema is called public so all the tables are public.canonical_fields, public.applications, etc.

@bickelj
Copy link
Contributor

bickelj commented Jul 12, 2022

@slifty Perhaps one advantage of separate schemas for isolation would be re-use of the connection pool by all tests. Then again, that's a kind of test non-isolation, so it could be a drawback. It would probably be cleanest to have a separate single connection to a separate single database for each test.

@slifty
Copy link
Member Author

slifty commented Jul 12, 2022

Ahhhhh got it -- that would be truly excellent.

I'll check out. At the very least using a separate schema within a database is not having to give "create db" permissions to the test user.

ty for raising it!

slifty added a commit that referenced this issue Jul 13, 2022
Integration tests are slower than unit tests because they rely on
a database connection, so we want the ability to run them separately
depending on our environment.

Jest allows us to define different config files.  By creating a `base`
config we can define all of the common settings between unit and
integration tests, and then customize for each type in the respective
extended configs.

In addition to configuration this begins to define define some hooks
that make it possible to run tests in parallel.  We can't actually use
these hooks properly until an upstream bug is fixed in our migration
package:

ThomWright/postgres-migrations#92

Issue #43 Support integration tests
slifty added a commit that referenced this issue Jul 13, 2022
Integration tests are slower than unit tests because they rely on
a database connection, so we want the ability to run them separately
depending on our environment.

Jest allows us to define different config files.  By creating a `base`
config we can define all of the common settings between unit and
integration tests, and then customize for each type in the respective
extended configs.

In addition to configuration this begins to define define some hooks
so that we can eventually run these tests in parallel.  Unfortunately
there is a bug in the migration library we're using which prevents that
kind of parallel migration within a single database / across multiple
schemas.  We have an open PR with a patch for that bug[1].

[1] ThomWright/postgres-migrations#93

Issue #43 Support integration tests
slifty added a commit that referenced this issue Jul 13, 2022
Integration tests are slower than unit tests because they rely on
a database connection, so we want the ability to run them separately
depending on our environment.

Jest allows us to define different config files.  By creating a `base`
config we can define all of the common settings between unit and
integration tests, and then customize for each type in the respective
extended configs.

In addition to configuration this begins to define define some hooks
so that we can eventually run these tests in parallel.  Unfortunately
there is a bug in the migration library we're using which prevents that
kind of parallel migration within a single database / across multiple
schemas.  We have an open PR with a patch for that bug[1].

[1] ThomWright/postgres-migrations#93

Issue #43 Support integration tests
slifty added a commit that referenced this issue Jul 13, 2022
Integration tests are slower than unit tests because they rely on
a database connection, so we want the ability to run them separately
depending on our environment.

Jest allows us to define different config files.  By creating a `base`
config we can define all of the common settings between unit and
integration tests, and then customize for each type in the respective
extended configs.

In addition to configuration this begins to define define some hooks
so that we can eventually run these tests in parallel.  Unfortunately
there is a bug in the migration library we're using which prevents that
kind of parallel migration within a single database / across multiple
schemas.  We have an open PR with a patch for that bug[1].

[1] ThomWright/postgres-migrations#93

Issue #43 Support integration tests
slifty added a commit that referenced this issue Jul 13, 2022
Integration tests are slower than unit tests because they rely on
a database connection, so we want the ability to run them separately
depending on our environment.

Jest allows us to define different config files.  By creating a `base`
config we can define all of the common settings between unit and
integration tests, and then customize for each type in the respective
extended configs.

In addition to configuration this begins to define define some hooks
so that we can eventually run these tests in parallel.  Unfortunately
there is a bug in the migration library we're using which prevents that
kind of parallel migration within a single database / across multiple
schemas.  We have an open PR with a patch for that bug[1].

I took out the explicit running of migrations because our integration
tests should now cover that (as they do run migrations).

[1] ThomWright/postgres-migrations#93

Issue #43 Support integration tests
slifty added a commit that referenced this issue Jul 15, 2022
Integration tests are slower than unit tests because they rely on
a database connection, so we want the ability to run them separately
depending on our environment.

Jest allows us to define different config files.  By creating a `base`
config we can define all of the common settings between unit and
integration tests, and then customize for each type in the respective
extended configs.

In addition to configuration this begins to define define some hooks
so that we can eventually run these tests in parallel.  Unfortunately
there is a bug in the migration library we're using which prevents that
kind of parallel migration within a single database / across multiple
schemas.  We have an open PR with a patch for that bug[1].

I took out the explicit running of migrations because our integration
tests should now cover that (as they do run migrations).

[1] ThomWright/postgres-migrations#93

Issue #43 Support integration tests
@bickelj
Copy link
Contributor

bickelj commented Jul 20, 2022

I have been trying to get the tests to fail properly when there is a connection failure to the db. On main, I get a timeout failure followed by "there are asynchronous operations that weren't stopped in your tests." I have tried a couple of things but it looks like beforeEach is looking for synchronous code, etc. My noobish summary is that I don't think the errors are coming back up the call stack, they're just out there somewhere until some timeouts hit.

@bickelj
Copy link
Contributor

bickelj commented Jul 20, 2022

See 5bdd199#r78988507

@bickelj
Copy link
Contributor

bickelj commented Jul 20, 2022

I have some beforeEach and afterEach code but it seems like they ignore any errors and continue to execute the test (e.g. timeout instead of exiting and failing). I wonder if jestjs/jest#2713 is still relevant.

@slifty
Copy link
Member Author

slifty commented Jul 20, 2022

Hmmm interesting -- this does sound pesky.

I wonder if there's a way to (A) signal that beforeEach is async and (B) fail a test from within beforeEach if the db connection is not formed.

I don't think we can avoid the timeout issue (though we could make it easier to configure timeout so that tests can configure a shorter timeout)

meta side note -- what do you think about opening a new issue just for this specific problem (I think we technically have implemented this issue as written).

@bickelj
Copy link
Contributor

bickelj commented Jul 21, 2022

I have peppered my local modified version of some code associated with PR #31 with logger.debug statements. I think you summarized it well as (A) having to do with async functions and (B) having to do with errors during beforeEach, but also it looks like something to do with the behavior of (C) TinyPg/Pg.Pool/the-way-we-manage-the-pool/clients. I will create an issue more precisely describing the issue.

@bickelj
Copy link
Contributor

bickelj commented Jul 21, 2022

Created #53 with some snippets.

@slifty
Copy link
Member Author

slifty commented Jul 21, 2022

Thank you! I have one small patch I'm going to make to this issue (the bug you found notwithstanding) -- which is to have the harness functions called by the suite (apparently Jest stops test flow if there is a failure, and so afterEach and beforeEach really do need to be specified to ensure everything is set up and cleaned up regardless of test outcome.

slifty added a commit that referenced this issue Jul 21, 2022
Manually calling the DB setup / teardown within a test was verbose, but
more importantly it meant teardown wouldn't happen if a test failed.
That's no good!

By using Jest's suite functionality we ensure that setup and teardown
both run for each test regardless of test outcome.

Issue #43 Support integration tests
slifty added a commit that referenced this issue Jul 21, 2022
Manually calling the DB setup / teardown within a test was verbose, but
more importantly it meant teardown wouldn't happen if a test failed.
That's no good!

By using Jest's suite functionality we ensure that setup and teardown
both run for each test regardless of test outcome.

Issue #43 Support integration tests
slifty added a commit that referenced this issue Jul 21, 2022
Manually calling the DB setup / teardown within a test was verbose, but
more importantly it meant teardown wouldn't happen if a test failed.
That's no good!

By using Jest's suite functionality we ensure that setup and teardown
both run for each test regardless of test outcome.

Issue #43 Support integration tests
slifty added a commit that referenced this issue Jul 21, 2022
Manually calling the DB setup / teardown within a test was verbose, but
more importantly it meant teardown wouldn't happen if a test failed.
That's no good!

By using Jest's suite functionality we ensure that setup and teardown
both run for each test regardless of test outcome.

Issue #43 Support integration tests
slifty added a commit that referenced this issue Jul 21, 2022
Manually calling the DB setup / teardown within a test was verbose, but
more importantly it meant teardown wouldn't happen if a test failed.
That's no good!

By using Jest's suite functionality we ensure that setup and teardown
both run for each test regardless of test outcome.

Issue #43 Support integration tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

2 participants