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

test(integration): add integration test setup and url tests #2019

Merged
merged 4 commits into from
Mar 14, 2023

Conversation

halfwhole
Copy link
Collaborator

@halfwhole halfwhole commented Oct 10, 2022

Problem

Integration tests are important. With the new external API feature coming up, it would be especially helpful to have greater assurances about the behaviour of the backend as a whole, and be able to catch and prevent regressions before they happen.

Right now, our backend is covered by unit tests; however, they make use of extensive mocking and do not test the real aggregate behaviour of the backend system. Our E2E tests help to cover for this somewhat, but they cannot be used to test our internal or upcoming external APIs directly; plus, they're expensive to run and cannot be used to comprehensively test edge cases and failure behaviours.

Solution

Set up integration tests in /test/integration/. These are written and run using the Jest framework.

As a start, a few integration tests have been added to test the internal API URL functionalities in Urls.test.ts:

  • Get URLs
  • Create URLs (link and file)
  • Update URLs (link and file)

To run the integration tests, run npm run test:integration.

Configuration

  • To prevent database conflicts across tests, each test case separately creates and uses a different randomly generated email of the format integration-[0-9a-z]{10}@open.gov.sg
    • We connect to the local database and write raw SQL to reset the users and their URLs
    • There's no data seeding yet, but this should be easy to add in the future by extending the SQL scripts
  • We wait 270s for docker-compose to finish spinning up the local environment before running integration tests
    • Not sure why the waiting time is set at exactly 270s, but I took this number from E2E tests which also sets it at 270s - these numbers should be consistent

Handling the database

  • Current approach: reuse the local database (as set up in docker-compose.yml) for integration tests
    • This is what's currently being done for E2E tests as well
    • Slight problem: this approach is not the cleanest, because the same database is also used for local development. But doing this will not affect staging/prod anyway, and practically it will not affect development much either
  • Other alternatives considered: 1) setting up a new database for testing only in the same postgres instance; 2) setting up separate postgres instance entirely for testing only
    • Problem: too costly to set up - it's difficult to link up sequelize to the new database or postgres instance, and create the tables correctly
    • Ideally, it would also be nice to access our models through sequelize ORM rather than raw queries, but again this is costly to set up

Other notes

  • I find it very sad to have to attach authCookie to every request, but I simply couldn't figure out how to make auth work automatically with cross-fetch on the backend even after wrestling with it for hours. Maybe axios would be better?
  • Actually, now that we use a different user and authCookie for each test case, it makes sense to attach it to every request. But the difficulty with using cross-fetch still stands :'(

Deploy Notes

ci.yml has been modified to include integration tests. Deployment to Elastic Beanstalk and Lambda will happen only if the integration tests pass (alongside CI and other Jest tests).

New dev dependencies:

  • form-data : used in integration tests to create FormData objects for creating file links

@halfwhole halfwhole marked this pull request as ready for review October 10, 2022 09:23
test/integration/util/helpers.ts Outdated Show resolved Hide resolved
test/integration/Urls.test.ts Outdated Show resolved Hide resolved
@halfwhole halfwhole force-pushed the test/integration/urls branch from 49bf773 to 87a440c Compare November 17, 2022 07:00
@halfwhole
Copy link
Collaborator Author

have changed the tests such that each test case can be run independently - it uses its own randomly generated user, which is created before the test case and removed after. please take a look!

@halfwhole halfwhole requested a review from thanhdatle November 17, 2022 07:04
@halfwhole halfwhole force-pushed the test/integration/urls branch from 87a440c to 4481243 Compare November 22, 2022 08:36
Copy link
Contributor

@gweiying gweiying left a comment

Choose a reason for hiding this comment

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

lgtm!

@halfwhole halfwhole force-pushed the test/integration/urls branch from 4481243 to c820607 Compare March 14, 2023 05:41
@gweiying gweiying mentioned this pull request Mar 14, 2023
@halfwhole halfwhole merged commit 957ff1e into develop Mar 14, 2023
@halfwhole halfwhole deleted the test/integration/urls branch March 14, 2023 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants