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

Create unit tests for validation #71

Closed
wants to merge 22 commits into from
Closed

Create unit tests for validation #71

wants to merge 22 commits into from

Conversation

zachclark-ccpo
Copy link
Contributor

@zachclark-ccpo zachclark-ccpo commented Aug 13, 2021

Overview

Using jest to test the business logic of the API endpoints.
Moved all of the validation to utils/validation.ts, each microservice will use these validation fns.
npm test output:

zclark@EDC-H6K8BH2:~/test-kyle-pr/atat-web-api/applications/portfolioDrafts$ npm test

> [email protected] test
> jest --config=jest.config.js

 PASS  test/requestbody.test.ts
  Testing validation of request body
    ✓ should return true because request body is present (2 ms)
    ✓ should return false because request body is empty JSON (1 ms)
    ✓ should return false when body is empty object (1 ms)
    ✓ should return false because request body is an empty String (1 ms)
    ✓ should return false because request body is empty whitespace String
    ✓ should return false because request body is null
    ✓ should return false because request body JSON is invalid (2 ms)
    ✓ should return true because request body JSON is valid
  Validation tests for createPortfolioStep function
    ✓ should map body to portfolioStep object
    ✓ should fail to map body to portfolioStep object due to missing attribute (2 ms)
    ✓ should fail to map body to portfolioStep object because request body is null
  Testing validation of path parameter
    ✓ should return false because path parameter is an empty string (1 ms)
    ✓ should return true because path parameter is a valid string (1 ms)
    ✓ should return false because path parameter is undefined (1 ms)

---------------|---------|----------|---------|---------|-------------------
File           | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
---------------|---------|----------|---------|---------|-------------------
All files      |     100 |      100 |     100 |     100 |                   
 validation.ts |     100 |      100 |     100 |     100 |                   
---------------|---------|----------|---------|---------|-------------------
Test Suites: 1 passed, 1 total
Tests:       14 passed, 14 total
Snapshots:   0 total
Time:        2.934 s, estimated 3 s
Ran all test suites.

TODO

  • Ensure all files are being collected with code coverage (Im certain that some of the fns outside of validation.ts).
  • Split up tests into smaller files
  • Fix lerna errors

First one added is portfolioStepCommand, placing it into its own file
so we can unit test the creation of the commands and test them against
a mock dynamo table
unit tests for various validation methods, aswell as object
mapping for the portfolioStep object
- isBodyPresent boolean valdiation to check for valid JSON but empty
request body
- isPathParameterPresent, same thing but for the path parameter
now using isPathParameterPresent and isBodyPresent
- add jest test script
- add jest dependency
- add ts-jest dependency
Used for jest tests, matches any file with '.test.ts' file extension
* @param body - The body of the request
* @returns true if the string is empty or null
*/
export function isBodyPresent(body: string | null): body is string {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is may be the wrong use of a type guard. It does accomplish the goals we have and it does in fact assert that the input is a string. But it fails to capture the additional restrictions imposed within the body.

zachclark-ccpo and others added 3 commits August 13, 2021 13:23
Before: string | undefined
Changed to: string
The actual UpdateCommand will not handle an undefined table name,
so it must be removed from the portfolioStepCommand
This will be used for unit testing locally with aws-sdk-mock
Not as useful as other mock tests, because we are not too concerned
if the object actually updates. But this proves a simple implementation of
local mocking.
This will allow us to unit test the getPortfolioStep dynamodb call,
and improves function clarity

const TABLE_NAME = process.env.ATAT_TABLE_NAME;
const TABLE_NAME = process.env.ATAT_TABLE_NAME ?? "";
Copy link
Contributor

Choose a reason for hiding this comment

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

If this isn't set, we need to totally abort. We can't work with a default value. This would be an unexpected infrastructure issue. We probably want a helper that does something like:

if (!process.env.ATAT_TABLE_NAME) {
  throw new Error("Oh no! The table name isn't set.")
}

of course with a better message and maybe a subclass of Error? But this really is an exceptional case that should never happen and if it does, we need to return a 500 response with a really good log message ASAP.

.gitignore Outdated Show resolved Hide resolved
- deleted unused imports
- refined test data
- mock GetCommand for getPortfolioStep
- this ensures that getPortfolioStepCommand is a proper
GetCommand (returns Item)
Copy link

sonarcloud bot commented Dec 6, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot E 1 Security Hotspot
Code Smell A 2 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

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.

2 participants