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

#2322 - IER12 E2E Tests - Validate Multiple Files #2478

Merged

Conversation

andrewsignori-aot
Copy link
Collaborator

@andrewsignori-aot andrewsignori-aot commented Nov 4, 2023

  • Changed the saveIER12TestInputData to use provided locations instead of creating a brand new one every time.
  • Once the institutions/locations were no longer brand new every time there were conflicts with the SABC program code. The logic was adapted to allow reusing the programs for the same institution and the same SABC code instead of creating new programs every time (which will generate the conflict).
  • Added the scenario to allow multiple files to be generated from different institutions/locations.
    • Should generate 2 IER12 files for locations from different institutions using distinct institution codes.
  • Created getUploadedFiles to allow getting multiple uploaded files. The method getUploadedFile was kept for now to prevent further refactoring and also it can be used as a helper since most of the same we upload one file only.
  • Fix for a dynamic data assertion that was being treated as a constant which will make the E2E tests fail.

@andrewsignori-aot andrewsignori-aot marked this pull request as ready for review November 4, 2023 02:19
@andrewsignori-aot andrewsignori-aot changed the title #2322 - IER12 E2E Tests - New Scenarios - Part 2 #2322 - IER12 E2E Tests - Validate Multiple Files Nov 4, 2023
// an institution ensure that the relationship will be kept and
// another institution will not be generated.
const institution =
relations?.institutionLocation?.institution ?? relations.institution;
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

@@ -45,6 +45,7 @@ export function createFakeInstitutionLocation(
count: 4,
upcase: true,
});
institutionLocation.hasIntegration = options?.initialValue?.hasIntegration;
Copy link
Collaborator

Choose a reason for hiding this comment

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

did we miss this in the previous iterations? or why was this added now?

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 was not needed till now.

return uploadedFile;
return sshClientMock.put.mock.calls.map((call) => {
const uploadedFile = {} as UploadedFile;
const [input, remoteFilePath] = call;
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 show stopper, just a suggestion. I believe we can remove this const and pass it in the call itself.

return sshClientMock.put.mock.calls.map(([input, remoteFilePath]) => {

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We can, I did the change and I am still not sure which one is cleaner from a code perspective.
Either way, I am accepting the suggestion.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed.

@@ -14,6 +15,15 @@ export function numberToText(
return value.toString().padStart(options?.length ?? 10, "0");
}

/**
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this method used anywhere?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am not sure if I am following the question. This method was added in the previous PR and it is used in 18 places.
image

Copy link
Collaborator

Choose a reason for hiding this comment

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

got it

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.

Nice work @andrewsignori-aot , Please have a look on some doubts.

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

},
assessment: {
triggerType: AssessmentTriggerType.OriginalAssessment,
// Add o one hour to ensure the proper file upload order.
Copy link
Contributor

Choose a reason for hiding this comment

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

Add **o** one??

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed.

expectedRecords?: number;
},
): QueueProcessSummaryResult {
return {
summary: [
`The uploaded file: ${process.env.INSTITUTION_REQUEST_FOLDER}\\ZZZY\\IER_012_${timestamp}.txt`,
`The uploaded file: ${process.env.INSTITUTION_REQUEST_FOLDER}\\${options?.institutionCode}\\IER_012_${timestamp}.txt`,
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

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 👍 nice work. only a very minor comment.

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.

Looks good! Great work!

Copy link
Collaborator

@sh16011993 sh16011993 left a comment

Choose a reason for hiding this comment

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

Nice Work @andrewsignori-aot 👍

Copy link

sonarqubecloud bot commented Nov 6, 2023

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 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link

github-actions bot commented Nov 6, 2023

Backend Unit Tests Coverage Report

Totals Coverage
Statements: 18.17% ( 2341 / 12882 )
Methods: 9.28% ( 151 / 1628 )
Lines: 20.69% ( 2016 / 9744 )
Branches: 11.52% ( 174 / 1510 )

Copy link

github-actions bot commented Nov 6, 2023

E2E Queue Consumers Coverage Report

Totals Coverage
Statements: 76.94% ( 564 / 733 )
Methods: 71.59% ( 63 / 88 )
Lines: 78.75% ( 493 / 626 )
Branches: 42.11% ( 8 / 19 )

Copy link

github-actions bot commented Nov 6, 2023

E2E Workflow Workers Coverage Report

Totals Coverage
Statements: 52.62% ( 341 / 648 )
Methods: 50% ( 40 / 80 )
Lines: 56.85% ( 282 / 496 )
Branches: 26.39% ( 19 / 72 )

Copy link

github-actions bot commented Nov 6, 2023

E2E SIMS API Coverage Report

Totals Coverage
Statements: 55.53% ( 4084 / 7354 )
Methods: 52.88% ( 505 / 955 )
Lines: 60.25% ( 3310 / 5494 )
Branches: 29.72% ( 269 / 905 )

}),
]);
// Assert file output.
const [fileA, fileB] = getUploadedFiles(sftpClientMock);
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

expectedRecords?: number;
},
): QueueProcessSummaryResult {
return {
summary: [
`The uploaded file: ${process.env.INSTITUTION_REQUEST_FOLDER}\\ZZZY\\IER_012_${timestamp}.txt`,
`The uploaded file: ${process.env.INSTITUTION_REQUEST_FOLDER}\\${options?.institutionCode}\\IER_012_${timestamp}.txt`,
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

@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.

Looks Good 👍

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.

6 participants