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

#1864 - MSFAA Send File E2E Test Automation #1905

Merged
merged 19 commits into from
Apr 28, 2023

Conversation

andrewsignori-aot
Copy link
Collaborator

@andrewsignori-aot andrewsignori-aot commented Apr 25, 2023

  • Created the method getUploadedFile to allow access to the supposed-to-be uploaded file.
  • Configure tests debug to use the timezone as UTC, otherwise, the expected result will differ from the real ones.
  • Create a specific factory (saveMSFAATestInputData) to generate the MSFAA records that could be used for all the MSFAA file-related tests.
  • Centralized the processDate used for MSFAA records, MSFAA file name, and MSFAA sequence group.
  • Added an "order by" on the getPendingMSFAARequest to ensure that the records are consistently retrieved in the same order and as a consequence, the file records will be generated using the same order also.
  • Created the helpers createE2EDataSources to be used specifically for E2E tests and reduce the amount of code needed when saving to DB multiple entities. The intention is to be used as below. Open for discussions and opinions.
    image
    This is related to the idea shared on the DEVs chat on April 11th
    image

@andrewsignori-aot andrewsignori-aot marked this pull request as ready for review April 26, 2023 21:41
Copy link
Contributor

@ann-aot ann-aot left a comment

Choose a reason for hiding this comment

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

LGTM @andrewsignori-aot 👍 Added some minor comments

gender: string;
maritalStatus: RelationshipStatus;
addressLine1: string;
addressLine2: string;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looking at the file-details, few of these inputs could be optional right? May be that could become a scenario in future if needed

image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

They can be test scenarios but not necessarily E2E scenarios. The idea is to have some E2E and focus more on the unit tests for the queue consumers, as per my understanding, to compensate also the decision to have E2E running sequentially.

Comment on lines +69 to +73
newMSFAANumber.dateRequested = null;
newMSFAANumber.dateSigned = null;
newMSFAANumber.serviceProviderReceivedDate = null;
newMSFAANumber.cancelledDate = null;
newMSFAANumber.newIssuingProvince = null;
Copy link
Collaborator

@dheepak-aot dheepak-aot Apr 27, 2023

Choose a reason for hiding this comment

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

IMO, I feel like this logic can be kept inside the factory createFakeMSFAANumber based on an optional parameter provided to the factory. As it is setting bunch of values to null.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It follows the idea of having the factory create the record and adjust the test as needed.
Keep in mind that these "nulls" are also set once inside a "factory".
Is the idea to create options like "pending", "canceled", or "signed" states?

* part of the MSFAA file generation.
* @param db data source helper.
* @param msfaa test input data.
* @returns a saved MSFAA record that used th input
Copy link
Collaborator

Choose a reason for hiding this comment

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

please correct the return comment to "@return a saved MSFAA record that uses the input test data to be created."

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed.

"test:e2e:queue-consumers": "npm run migration:run && cross-env ENVIRONMENT=test jest --collect-coverage --verbose --config ./apps/queue-consumers/test/jest-e2e.json --forceExit",
"test:e2e:queue-consumers:local": "cross-env ENVIRONMENT=test jest --verbose --config ./apps/queue-consumers/test/jest-e2e.json --forceExit",
"test:e2e:queue-consumers": "npm run migration:run && cross-env ENVIRONMENT=test jest --collect-coverage --verbose --config ./apps/queue-consumers/test/jest-e2e.json --runInBand --forceExit",
"test:e2e:queue-consumers:local": "cross-env ENVIRONMENT=test TZ=UTC jest --verbose --config ./apps/queue-consumers/test/jest-e2e.json --runInBand --forceExit",
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@github-actions
Copy link

Backend Unit Tests Coverage Report

Totals Coverage
Statements: 17.97% ( 1980 / 11017 )
Methods: 8.06% ( 115 / 1427 )
Lines: 20.75% ( 1736 / 8368 )
Branches: 10.56% ( 129 / 1222 )

@github-actions
Copy link

E2E Workflow Workers Coverage Report

Totals Coverage
Statements: 32.41% ( 176 / 543 )
Methods: 21.25% ( 17 / 80 )
Lines: 39.25% ( 157 / 400 )
Branches: 3.17% ( 2 / 63 )

@github-actions
Copy link

E2E Queue Consumers Coverage Report

Totals Coverage
Statements: 66.67% ( 362 / 543 )
Methods: 55.07% ( 38 / 69 )
Lines: 68.97% ( 320 / 464 )
Branches: 40% ( 4 / 10 )

Copy link
Collaborator

@andrepestana-aot andrepestana-aot left a comment

Choose a reason for hiding this comment

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

Great job! Looks good!

@github-actions
Copy link

E2E SIMS API Coverage Report

Totals Coverage
Statements: 39.13% ( 2651 / 6775 )
Methods: 31.56% ( 278 / 881 )
Lines: 44.53% ( 2248 / 5048 )
Branches: 14.78% ( 125 / 846 )

@@ -11,7 +11,7 @@ import {
overrideImportsMetadata,
} from "@sims/test-utils";
import * as Client from "ssh2-sftp-client";
import { createMock } from "@golevelup/ts-jest";
import { DeepMocked, createMock } from "@golevelup/ts-jest";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not a part of the PR, I have question on line 45
Will this automatically mock the client.put which uploads the file?
const sshClientMock = createMock<Client>();

image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, createMock<Client>(); is a helper to do a "deep" mock.

@dheepak-aot
Copy link
Collaborator

Thanks for setting up the E2E tests for file integration. Added few minor comments/questions.

Copy link
Collaborator

@dheepak-aot dheepak-aot left a comment

Choose a reason for hiding this comment

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

Thanks for making the code change and the walk through. LGTM 👍

Copy link
Contributor

@ann-aot ann-aot left a comment

Choose a reason for hiding this comment

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

Good work @andrewsignori-aot 👍

@@ -102,7 +104,7 @@ export class MSFAARequestProcessingService extends ESDCFileHandler {
const msfaaNumberRepo = entityManager.getRepository(MSFAANumber);
this.msfaaNumberService.updateRecordsInSentFile(
msfaaRecordIds,
getUTCNow(),
processDate,
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

Copy link
Collaborator

@guru-aot guru-aot left a comment

Choose a reason for hiding this comment

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

LGTM, nice work @andrewsignori-aot

@andrewsignori-aot andrewsignori-aot merged commit 47a4dce into main Apr 28, 2023
@andrewsignori-aot andrewsignori-aot temporarily deployed to DEV April 28, 2023 00:46 — with GitHub Actions Inactive
@andrewsignori-aot andrewsignori-aot deleted the feature/#1864-part-time-msfaa-e2e-test branch April 28, 2023 00:46
@andrewsignori-aot andrewsignori-aot temporarily deployed to DEV April 28, 2023 00:46 — with GitHub Actions Inactive
@andrewsignori-aot andrewsignori-aot temporarily deployed to DEV April 28, 2023 00:56 — with GitHub Actions Inactive
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.

5 participants