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

Remove more DatabaseSupport.clean() from services/job #1246

Merged
merged 2 commits into from
Aug 8, 2024

Conversation

jonathangoulding
Copy link
Collaborator

@jonathangoulding jonathangoulding commented Aug 8, 2024

DEFRA/water-abstraction-team#124

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, 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-licences to drop their dependence on DatabaseSupport.clean().

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, 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-licences to drop their dependence on DatabaseSupport.clean().
@jonathangoulding jonathangoulding self-assigned this Aug 8, 2024
@jonathangoulding jonathangoulding added the testing A change to the tests or CI label Aug 8, 2024
@jonathangoulding jonathangoulding marked this pull request as ready for review August 8, 2024 13:34
@jonathangoulding
Copy link
Collaborator Author

Appreciate this is a small one but the others in here where quite complex to tackle and i am going back to working on the import licence system tasks.

Copy link
Member

@Cruikshanks Cruikshanks left a comment

Choose a reason for hiding this comment

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

@jonathangoulding jonathangoulding merged commit 20ae3cd into main Aug 8, 2024
6 checks passed
@jonathangoulding jonathangoulding deleted the remove-database-clean-services-job branch August 8, 2024 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing A change to the tests or CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants