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

Creates Test Plan for Running Integration Tests on Server API Endpoints #1493

Merged
merged 31 commits into from
Nov 2, 2023

Conversation

agosmou
Copy link
Member

@agosmou agosmou commented Oct 26, 2023

Fixes #1400

Overview of Added Features

  • Integration Testing Plan, i.e. the Environment used for testing the TDM Server
  • CI for Testing
  • Accounts Endpoints Tests

What changes did you make?

  • Added Test Plan for use in running integration tests on the TDM Server:
    • Added 'Jest' to run tests
    • Implemented node library 'testcontainers' to create a docker container for an instance of a test MS SQL Server database
    • Added the integration tests to the TDM CI pipeline so they run on every pull request to the 'develop' branch
  • Added integration tests for the 'accounts' endpoints that test the functional requirements of the user managemnet workflow as well as edge cases
  • Updated the Test Wiki to include documentation on the new Test Plan and instructions to add future tests

Why did you make the changes (we will use this info to test)?

  • Reliable Testing: Integrated 'Jest' and 'testcontainers' to add robust testing reflecting our production setup
  • Enhanced CI/CD: Integration tests in TDM CI pipeline prevent PRs that merge breaking changes
  • User Workflow Quality: Added tests for 'accounts' endpoints to ensure user management workflow meets functional requirements and handles edge cases.

References

Jest

Supertest

Testcontainers

SendGrid (Mocked for Testing)

Flyway Migrations

  • Source Code observed to use in flyway config to disable reports when testing

Suggested Order for File Review (root directory would be server)

Environment Variables
0. .example.env

Setting up a containerized db with application code
1a. utils/mssql-container-setup.js
1b. db/flyway-config.js
1c. global-setup/global-setup.js
1d. global-setup/global-teardown.js

Server refactors and setup for use as an instance on tests
2a. server.js
2b. utils/server-setup.js
2c. local-setup/setup-after-env.js

Testing configs and script
3a. jest.config.js
3b. package.json

Accounts tests
4. __tests__/account.test.js

Setting up github actions
5. .github/workflows/tdm-server-tests.yml

Discovery Notes

WebAPI endpoints Testing Feature Requirements:

  • Maintainable
  • Scalable
  • Reliable

Considered Implementations:

  • Postman/Hoppscotch
    • Pros: It seemed like a good option to use a GUI for testing. Postman's user management and pricing is uncertain as well as it does not allow for more than 3 people to share a testing repo, but Hoppscotch does so I was exploring Hoppscotch as this is a project worked on by many devs and eventually LADOT. Hoppscotch also allows you to put your test suites into a json file that can be run using their CLI.
    • Cons: Hoppscotch still had some development left to do to include features like taking variables from one test, e.g. token, and applying them to another test. This was limiting.
  • Bash script with Curl commands
    • I tried this but it became a massive file that was difficult to update and interpret
  • Jest with a local db spun up by user
    • This seemed like a good option because It would work for testing in the CI pipeline as well, but relying on the dev to spin up their own container and db added a layer of friction when trying to incentivize testing. Additionally our use of stored procedures needed to be considered for the testing db platform.
  • Jest with Testcontainers library so db is integrated
    • This felt the most maintainable, reliable, and scalable - Everything is integrated into application code and dependencies are stable. It is build to easily add further tests without having to juggle other considerations.

@agosmou agosmou self-assigned this Oct 26, 2023
@agosmou agosmou added this to the 02 - Security milestone Oct 26, 2023
Copy link
Member Author

@agosmou agosmou left a comment

Choose a reason for hiding this comment

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

Added comments to files to explain

Copy link
Member Author

Choose a reason for hiding this comment

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

OBSOLETE - This was a script to test using Hoppscotch

Copy link
Member Author

Choose a reason for hiding this comment

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

renamed file

# APPLICATIONINSIGHTS_CONNECTION_STRING=

###############################################################
## Testing (Comment out ALL the above variables) ##
Copy link
Member Author

Choose a reason for hiding this comment

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

renamed file with better naming convention and updated the .env file layout so that users can test locally by simply commenting out the development variables and un-commenting the testing variables.

I will have to add an updated .env file to the google drive upon accepted changes

await teardownServer();
});

beforeEach(() => {
Copy link
Member Author

Choose a reason for hiding this comment

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

This mocks the sendgrid function call on the sendgrid client to avoid costs incurred from calling 3rd party API's

Documentation used to produce this sendgrid mock:

  • I had to mock the sendgrid API because they dont have a test key. If I used their SandboxMode, it would only validate the data structure of the request so I wouldnt be able to intercept the request and grab the token for the email confirmation on the integration tests

References

teardownServer
} = require("../_jest-setup_/utils/server-setup");

let server;
Copy link
Member Author

Choose a reason for hiding this comment

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

the supertest library allows you to pass a real server instance. I chose to do this to mimic production more closely.

This is an optional pattern and you are able to avoid instantiating the server - supertest will use an in-memory representation. Simply remove the setupServer(); function call.

@@ -28,7 +28,8 @@ module.exports = function () {
sqlMigrationSuffixes: ".sql",
baselineOnMigrate: true,
baselineVersion: "0002", // Do not change this baseline version number
baselineDescription: "setup_db_baseline_data_as_of_07012020"
baselineDescription: "setup_db_baseline_data_as_of_07012020",
reportEnabled: process.env.TEST_ENV ? false : true
Copy link
Member Author

Choose a reason for hiding this comment

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

Running the flyway migrations creates a multi-thousand line report - this code prevents this from happening during testing but preserves this function during development and production environments

Copy link
Member Author

Choose a reason for hiding this comment

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

configurations for jest

Copy link
Member Author

Choose a reason for hiding this comment

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

This adds supertest, jest, and testcontainers. These are popular libraries that are well maintained with stable dependencies.

@@ -9,7 +9,7 @@
"main": "server.js",
"scripts": {
"precommit": "lint-staged",
"test": "jest --passWithNoTests",
"test": "jest --runInBand --passWithNoTests --forceExit",
Copy link
Member Author

Choose a reason for hiding this comment

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

"--runInBand" is added to run the test files sequentially rather than in parallel because they are sharing a database instance.

"--forceExit" is a measure to force jest to exit in the case there is a server that is hanging up from being improperly closed.

@@ -75,6 +75,8 @@ app.use((err, req, res) => {

app.use(errorHandler);

app.listen(port, () => console.log(`Server running on port ${port}`));
if (process.env.TEST_ENV !== "true") {
Copy link
Member Author

Choose a reason for hiding this comment

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

Added a minor conditional to be able to instantiate the server, i.e. set them up and tear them down. Development environments and production environments will have a falsy conditional; thus, be unaffected.

this was a necessary change to be able run multiple test suites with a real server.

@agosmou
Copy link
Member Author

agosmou commented Oct 26, 2023

I tried to keep commits atomic, but there was A LOT of refactoring; hence, the large amount of commits. We can squash and merge to condense this.

@agosmou
Copy link
Member Author

agosmou commented Oct 26, 2023

Wiki is linked on comment above. Do review for further info and provide feedback as this is the "source of truth" to keep the test plan readable, maintainable, and scalable.

@agosmou agosmou marked this pull request as ready for review October 26, 2023 23:31
@agosmou
Copy link
Member Author

agosmou commented Oct 26, 2023

@entrotech
I shared the .env for this file on the shared drive so you can use it in your local environment when reviewing.

alternatively you can grab the env variables from the github actions file - just make sure all other variables in your .env are commented out!

@agosmou
Copy link
Member Author

agosmou commented Oct 30, 2023

@entrotech

The below should facilitate interpretation of code


Suggested Order for File Review (root directory would be server):

Environment Variables
0. .example.env

Setting up a containerized db with application code
1a. utils/mssql-container-setup.js
1b. db/flyway-config.js
1c. global-setup/global-setup.js
1d. global-setup/global-teardown.js

Server refactors and setup for use as an instance on tests
2a. server.js
2b. utils/server-setup.js
2c. local-setup/setup-after-env.js

Testing configs and script
3a. jest.config.js
3b. package.json

Accounts tests
4. __tests__/account.test.js

Setting up github actions
5. .github/workflows/tdm-server-tests.yml

@entrotech entrotech merged commit a77887d into develop Nov 2, 2023
1 check passed
@agosmou agosmou deleted the 1400-ag-accounts-api-tests branch November 11, 2023 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Engineer tests and validation for 'Account' Web API requests
2 participants