[no-issue]: Convert database tests to jest #4188
Conversation
| @@ -101,17 +99,5 @@ describe('DataElement', () => { | |||
| ); | |||
| }); | |||
| }); | |||
|
|
|||
| it('should remove valid fields when empty', () => { | |||
There was a problem hiding this comment.
This was a duplicate test case wtih a misleading name
kael89
force-pushed
the
convert-database-tests-to-jest
branch
from
e744b7a
to
edf3d4e
Compare
There was a problem hiding this comment.
Looked through and all looks good to me! Great that you caught a few things like that dupe test, some unneeded awaits, and even the language used to describe each test case.
Thanks for swooping in with your tech debt cleaning super powers, Open Source Community 😁
| runSql: (query, replacementParams) => ({ query, replacementParams }), | ||
| }; | ||
|
|
||
| describe('migrationUtilities', () => { |
There was a problem hiding this comment.
Restructured this file a bit to make it less verbose
|
Have enabled auto-merge, will be committed to dev as soon as CI/CD checks have passed. |
Ah nice, that's a handy feature! A few dev scripts have been touched because of the test folder update. I guess that if anything has been messed up the CI/CD will tell :-) |
| // responseDescriptionFields, | ||
| // )}`; | ||
|
|
||
| expect(surveyResponse).toHaveProperty('outdated', expected); |
There was a problem hiding this comment.
Using a 3rd parameter (for a custom message) is not only unsupported by jest, but it causes toHaveProperty to always pass! 😨
Interestingly this is not the case with toBe, which fails as expected. I double-checked that the only instance where custom messages are still used in this package is in AnalyticsRefresher.test.js, for .toBe assertions.
2 more packages (
central-server,web-config-server) and thenmochacan go! 🎉Most of the job was done using a community codemod