Remove more DatabaseSupport.clean() from tests #1241
Conversation
We had a pattern of calling `await DatabaseSupport.clean()` in a `beforeEach()` in any unit test file that depended on adding data to support its tests. The problem is that the project is becoming so large that it is no longer sustainable. It is also a blocker to moving to a test framework that supports [ECMAScript Modules](https://nodejs.org/api/esm.html), [the official standard format](https://tc39.github.io/ecma262/#sec-modules) to package JavaScript code. Unit test frameworks other than [Hapi Lab](https://hapi.dev/module/lab/) run tests in parallel by default. This means you can't have one test running DB clean while another is trying to load data. New tests avoid the pattern, and where we can, we've been updating old ones when we have had cause to update the unit tests. But there are always those where it'll be some time before we have cause to look at them again. So, rather than waiting, this change handles updating all tests in `test/services/bill-licences` to drop their dependence on `DatabaseSupport.clean()`.
|
Tested with the seed and clean commented out to ensure the test will work generically. |
| code: 'S127', | ||
| description: 'Section 127 (Two Part Tariff)' | ||
| }) | ||
| const section126 = await FinancialAgreementModel.query().select().where('code', 'S126').limit(1).first() |
There was a problem hiding this comment.
This is necessary / easy to manage duplicate insert for what was the same data.
If these can be seeder then this can use the seeder data files in the future.
There was a problem hiding this comment.
If this is okay.
Then reviewer can you resolve.
|
Most of the changes are the removal of the clean database code. Some changes have been minimally added to generate unique data i.e generateLicenceRef() / generateUUID(). The biggest changes are in the licence-abstraction-data.seeder.js. |
jonathangoulding
requested review from
Cruikshanks,
rvsiyad,
Jozzey,
robertparkinson and
Beckyrose200
Cruikshanks
changed the title
|
Sorry @jonathangoulding . I merged on your behalf because both this and my PR #1230 touch a lot of the same files. Having this merged means I can crack on and deal with the conflicts. |
DEFRA/water-abstraction-team#124
This change, like all things, initially started with small changes in the bills and licences service tests. As we moved on to the return requirements, most of them were straight-forward. However, some complexity was added with the licence-abstraction-data.seeder.js. Some temporary code has been introduced to create the financial agreements if they don't exist (they are queried and used if they do). This is necessary as some tests are still cleaning the database.
We had a pattern of calling
await DatabaseSupport.clean()in abeforeEach()in any unit test file that depended on adding data to support its tests.The problem is that the project is becoming so large that it is no longer sustainable. It is also a blocker to moving to a test framework that supports ECMAScript Modules, the official standard format to package JavaScript code. Unit test frameworks other than Hapi Lab run tests in parallel by default. This means you can't have one test running DB clean while another is trying to load data.
New tests avoid the pattern, and where we can, we've been updating old ones when we have had cause to update the unit tests.
But there are always those where it'll be some time before we have cause to look at them again. So, rather than waiting, this change handles updating all tests in
test/services/bill-licencesto drop their dependence onDatabaseSupport.clean().