Backend tests too brittle.

[rahearn]

Looks like we're running into a lot of issues with tests clobbering the DB for other tests.

adding --runInBand might be a bandaid for this issue, but I'd like to solve the root issue for the long term.

https://app.circleci.com/pipelines/github/HHS/Head-Start-TTADP/165/workflows/bae0eaf5-5821-4682-940b-837dc796c434/jobs/922

[dcloud]

Just started taking a look at this. It sounds like people are seeing errors in tests that rely on the database, and the thinking is that one tests's db transactions aren't completing before another db-dependent test runs, or something to that effect.

The evidence in CircleCI, based on the test run linked above, is errors like:

SequelizeForeignKeyConstraintError: insert or update on table "Permissions" violates foreign key constraint "Permissions_userId_fkey"

It sounds like in that case the errors are related to creation of users and permissions, and another test run (jobs/923) shows those same errors. An earlier test run (jobs/920) also shows db errors, but for foreign key constraints on ActivityReports.

So we need to work on guaranteeing that db is in a "clean and ready" state at the beginning of each tests. It also sounds like this may only be happening in CircleCI? Need to investigate how to reproduce, and how to make sequelize and Jest play nice with each other (and CircleCI, probably)

[rahearn]

I think it's happening locally too, though perhaps less often? It's definitely a bit of a race condition though so it's difficult to get a consistent reproduction.

I think there must be some best-practices around test setup that we're not doing that are likely all that are needed here. In a rails app this would be handled with wrapping tests in transactions to rollback any changes after the test finishes.

[dcloud]

Yeah, it looks like this does happen locally, and that might be more complicated, since the dbdata volume persists between runs. Also noticed that yarn run docker:test:be doesn't use the same docker-compose-test.yml that circleci uses. Going to rename that docker-compose file used for security scans.

I agree that using transactions might be part of the path forward. I'll be looking at that in a bit.

[rahearn]

dbdata volume persists

one thing that could be a good first step is just using a different db instance for dev vs test environments

[dcloud]

The different configs for dev vs test turned into a bit of a rabbit hole with respect to creating multiple docker-compose.*.yml files and wiring things up as yarn commands. Going to stash that for now and come back to it (which will probably involve making bash scripts to encapsulate the docker-compose commands).

Heading back to transactions land and looking for any holes in my understanding of things, e.g. all the db:seed* commands seem to be very similar (and db:seed:prod may be a WIP?).

[dcloud]

First, the tests appeared to be failing in the same place, under src/routes/activityReports/handlers.test.js, but those tests were refactored to not be dependent on creating records in the database. The underlying functions, e.g. usersWithPermissions, that interact with the database are now mocked when testing the handlers, and those functions have their own tests that do create database records. I think one issue with the earlier tests might have been two promises not wrapped in a Promise.all().

At this point in time, I am seeing backend tests running repeatedly without failures, with --runInBand removed. I also don't see a blanket solution at the moment, though being careful about resolving all your promises (await-ed expressions) is a good piece of advice to follow.

Second, I wanted to note that the dbdata volume situation doesn't exist in CircleCI, so it's of a lesser importance. I did spend time with composing docker-compose files, though I don't know if they added complexity is worthwhile. I may make a separate docker-compose.testing.yml rather than do composition which would require -f docker-compose.yml -f docker-compose.testing.yml at every docker-compose invocation.