-
Notifications
You must be signed in to change notification settings - Fork 14
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
#1724 - E2E/Units Tests - Review Queue Consumers E2E Tests #1872
#1724 - E2E/Units Tests - Review Queue Consumers E2E Tests #1872
Conversation
…ub.com/bcgov/SIMS into feature/#1724-queue-conumers-e2e-tests
There was a problem hiding this 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 👍 Only minor comments. Approving the PR
...ntegration/ecert-integration/tests/ecert-full-time-process-integration.scheduler.e2e-spec.ts
Outdated
Show resolved
Hide resolved
...packages/backend/apps/queue-consumers/test/helpers/testing-modules/testing-modules-helper.ts
Outdated
Show resolved
Hide resolved
...sumers/src/processors/assessment/_tests_/cancel-application-assessment.processor.e2e-spec.ts
Outdated
Show resolved
Hide resolved
...sumers/src/processors/assessment/_tests_/cancel-application-assessment.processor.e2e-spec.ts
Show resolved
Hide resolved
...sumers/src/processors/assessment/_tests_/cancel-application-assessment.processor.e2e-spec.ts
Show resolved
Hide resolved
...sumers/src/processors/assessment/_tests_/cancel-application-assessment.processor.e2e-spec.ts
Outdated
Show resolved
Hide resolved
Thanks for creating E2E test foundation for queue consumers. Added some comments. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job. I just added a question.
...egration/ecert-integration/_tests_/ecert-full-time-process-integration.scheduler.e2e-spec.ts
Outdated
Show resolved
Hide resolved
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this 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 changes and walking through the replace module code. LGTM 👍
There was a problem hiding this 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
createTestingAppModule
to start the queue-consumers module for all E2E tests.describeProcessorRootTest
with the intention to create a similar one for the scheduler in the future.@golevelup/ts-jest
referenced on Nestjs documentation (https://docs.nestjs.com/fundamentals/testing). The idea is to validate how useful it can be and assess if it is needed or not.start-application-assessment
andcancel-application-assessment
, as shown below.ecert-full-time-process-integration.scheduler.e2e-spec
is not the focus of this PR. It was just adjusted to fit better in the new E2E structure.Processor E2E tests for start-application-assessment
√ Should throw an error when the workflow createProcessInstance method throws an error.
√ Should invoke the workflow create instance method with the received job parameters.
Processor E2E tests for cancel-application-assessment
√ Should cancel pending disbursements and rollback overawards when the cancelled application has overawards and also one sent and one pending disbursements.
√ Should cancel workflow and log a warning when the workflowInstanceId is not present.
√ Should throw an error and call job.discard when the application is not in the expected status.
√ Should throw an error and call job.discard when the assessment id was not found.