Add notes on testing, remove --runInBand

docs/testing.md

@@ -0,0 +1,38 @@
+# Testing
+
+## Tips and Caveats when writing tests
+
+
+### Handling async/promises
+
+When writing tests that rely on asynchronous operations, such as writing to the database, take care to make sure that those operations are resolved before any tests that rely on them run. If you need to create database records in a setup function such as `beforeAll`, you will want to make sure all async/promise operations resolve before subsequent tests run. You can make sure multiple await (promise) operations resolve by using [`Promise.all()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/all) (which takes an iterable of promises).
+
+Here's how that might look:
+
+```
+let a = someAsyncFun();
+let b = anotherAsyncFun();
+
+return Promise.all([a, b]);
+```
+
+### Creating/deleting database records
+
+Some tests will require interactions with the database, but at present all tests run using the same instance of the database. That means that any records you create or delete can potentially affect tests elsewhere. On top of that, tests run in parallel, so database operations may run in an unexpected order. That can mean tests may pass various times only to fail due to missing or unexpected records in the database when your tests run.
+
+To mitigate issues with missing or unexpected records causing failing tests, you can try a few approaches. One approach is to avoid using the database if your test doesn't actually require it. You may be able to use mock models or responses rather than interact with the database. If your test does require the database, you should create the records you need before your tests run and delete the records you created (and no others) when your tests finish. If you `Model.create`, make sure you `Model.destroy()` the records you created.
+
+When writing tests that create database records, it might also help to use a `try...catch` to catch errors in database transactions and log meaningful output. Sequelize error messages can be vague, and it might help others to see more informative messages.
+
+## Testing in Docker
+
+When running tests in Docker, be aware that there are tests that will modify/delete database records. For tests to run, the 'db' service needs to exist and `db:migrate` and `db:seed` need to have been run (to create the tables and populate certain records).
+
+In the `docker-compose.yml` configuration, the database is set up to persist to a volume, "dbdata", so database records will persist between runs of the 'db' service, unless you remove that volume explicitly (e.g. `docker volume rm` or `docker-compose down --volumes`).
+
+
+### Notes on docker-compose and multiple configurations
+
+`docker-compose` has a feature for providing multiple `docker-compose.*.yml` files where subsequent files can override settings in previous files, which sounds like it would suit the use case of running docker for local development and for testing. However, the ability to [override configurations](https://docs.docker.com/compose/extends/#adding-and-overriding-configuration) is limited. While experimenting with overrides, it became clear that doing so would require a minimum of three docker-compose.yml files: one "base", one for local development, one for running tests. Trying to compose docker-compose.yml files would be complicated.
+
+In addition, while experimenting with multiple configuration files, it became clear that docker was unable to differentiate between different versions of the same service. Trying to override the 'db' service for testing would not work as expected: if the local/dev 'db' service had already been created, that one would be used when tests were run.

package.json

@@ -18,7 +18,7 @@
     "server:debug": "nodemon src/index.js --exec babel-node --inspect",
     "client": "yarn --cwd frontend start",
     "test": "jest src",
-    "test:ci": "cross-env JEST_JUNIT_OUTPUT_DIR=reports JEST_JUNIT_OUTPUT_NAME=unit.xml POSTGRES_USERNAME=postgres POSTGRES_DB=ttasmarthub CURRENT_USER_ID=5 CI=true jest src --runInBand --coverage --reporters=default --reporters=jest-junit",
+    "test:ci": "cross-env JEST_JUNIT_OUTPUT_DIR=reports JEST_JUNIT_OUTPUT_NAME=unit.xml POSTGRES_USERNAME=postgres POSTGRES_DB=ttasmarthub CURRENT_USER_ID=5 CI=true jest src --coverage --reporters=default --reporters=jest-junit",
     "test:all": "yarn test:ci && yarn --cwd frontend test:ci",
     "lint": "eslint src",
     "lint:ci": "eslint -f eslint-formatter-multiple src",

src/routes/admin/user.test.js

@@ -55,7 +55,10 @@ describe('User route handler', () => {
     jest.clearAllMocks();
   });
   afterAll(async () => {
-    await User.destroy({ where: {} });
+    await User.destroy({ where: { id: 49 } });
+    await User.destroy({ where: { id: 50 } });
+    await User.destroy({ where: { id: 52 } });
+    await User.destroy({ where: { id: 53 } });
     db.sequelize.close();
   });
 

src/routes/files/handlers.test.js

@@ -16,21 +16,21 @@ const ORIGINAL_ENV = process.env;
 jest.mock('../../lib/s3Uploader');
 
 const mockUser = {
-  id: process.env.CURRENT_USER_ID,
+  id: 100,
   homeRegionId: 1,
   permissions: [
     {
-      userId: process.env.CURRENT_USER_ID,
+      userId: 100,
       regionId: 5,
       scopeId: SCOPES.READ_WRITE_REPORTS,
     },
     {
-      userId: process.env.CURRENT_USER_ID,
+      userId: 100,
       regionId: 6,
       scopeId: SCOPES.READ_WRITE_REPORTS,
     },
     {
-      userId: process.env.CURRENT_USER_ID,
+      userId: 100,
       regionId: 14,
       scopeId: SCOPES.SITE_ACCESS,
     },
@@ -63,6 +63,7 @@ describe('File Upload', () => {
       report = await ActivityReport.create(reportObject);
       process.env.NODE_ENV = 'test';
       process.env.BYPASS_AUTH = 'true';
+      process.env.CURRENT_USER_ID = 100;
     });
     afterAll(async () => {
       await File.destroy({ where: {} });
@@ -104,6 +105,7 @@ describe('File Upload', () => {
       report = await ActivityReport.create(reportObject);
       process.env.NODE_ENV = 'test';
       process.env.BYPASS_AUTH = 'true';
+      process.env.CURRENT_USER_ID = 100;
     });
     afterAll(async () => {
       await File.destroy({ where: {} });