#1722 - E2E Tests for assessment gateway #1822
Conversation
dheepak-aot
changed the title
dheepak-aot
requested review from
guru-aot,
sh16011993,
JasonCTang,
andrepestana-aot,
andrewsignori-aot and
ann-aot
| zeebeClientProvider = ZeebeMockedClient.getMockedZeebeInstance(); | ||
| }); | ||
|
|
||
| it("Should follow the expected workflow path when student single and independent without application exception and PIR.", async () => { |
There was a problem hiding this comment.
when student is single and independent without?
| programYear: string, | ||
| ): AssessmentConsolidatedData { | ||
| const [, programEndYear] = programYear.split("-"); | ||
| const assessmentConsolidatedData = |
There was a problem hiding this comment.
I have a qn here, next if we want to test a different set of info, for eg, TEST a "full time" assessment with a "dependent" status application, are we planning to make this method generic for all FT tests or are we planning to create "createFakeConsolidatedSOMETHINGFT". do we have a plan? or we are planning to figure it out on the way.
There was a problem hiding this comment.
The plan should be for e.g. TEST a "full time" assessment with a "dependent" if required only at a test case level, then it will be prepared at test case by using generic method and modifying it. Same if it is required at e2e test file level, then it can be prepared at beforeAll(), or else if it is required for multiple test files then createFakeConsolidatedSOMETHINGFT will be created at factory level.
There was a problem hiding this comment.
Also, there is nothing against creating a new createFakeAssessmentConsolidatedData with some initial data as a baseline, for instance.
createFakeAssessmentConsolidatedDataForTwoParentscreateFakeAssessmentConsolidatedDataWithDependentscreateFakeAssessmentConsolidatedDataWithPartnercreateFakeAssessmentConsolidatedDataForPIR
Or have all of them as options on thecreateFakeAssessmentConsolidatedData
createFakeAssessmentConsolidatedData(options: {
includeParent1SupportingData: boolean,
includeParent2SupportingData: boolean,
includePartnerSupportingData: boolean,
generateDependents: numbers
})
| // Based on this variable load consolidated data worker will | ||
| // return the application exception status. | ||
| [E2E_APPLICATION_EXCEPTION_STATUS]: | ||
| ApplicationExceptionStatus.Approved, |
There was a problem hiding this comment.
mm.. this is a happy path scenario, right? so why do we want to set this status? I may be missing something
There was a problem hiding this comment.
As we discussed, this is to simulate the happy path returned by the actual worker.
| }); | ||
| // Workflow instance expected to pass through associate workflow instance worker. | ||
| expect( | ||
| assessmentGatewayResponse.variables[Workers.AssociateWorkflowInstance], |
There was a problem hiding this comment.
I would not mind having these ones created at the lib level and shared between the worker's application also.
@ZeebeWorker(Workers.AssociateWorkflowInstance
sources/packages/backend/workflow/test/test-utils/constants/worker-constants.ts
Outdated
Show resolved
Hide resolved
| @@ -5,7 +5,7 @@ services: | |||
| environment: | |||
| - ZEEBE_BROKER_DATA_DISKUSAGECOMMANDWATERMARK=0.998 | |||
| - ZEEBE_BROKER_DATA_DISKUSAGEREPLICATIONWATERMARK=0.999 | |||
| - "JAVA_TOOL_OPTIONS=-Xms256m -Xmx256m" | |||
| - "JAVA_TOOL_OPTIONS=-Xms512m -Xmx512m" | |||
There was a problem hiding this comment.
Is it to reduce the memory consumption on the dev machine or GitHub action machines?
Can you please clarify which was the motivation around it?
There was a problem hiding this comment.
When I created the docker-compose exclusively for testing, provided 256m to keep it light. But considering the number of workflow scenarios now tested and will be tested in future, increased it to 512m to ensure stability.
There was a problem hiding this comment.
In the other PR I would comment on that. I don't know how it was possible to run that with only 256m 😄
| { | ||
| taskType: Workers.LoadAssessmentConsolidatedData, | ||
| taskHandler: (job) => | ||
| job.complete(getMockConsolidatedData(job.variables)), |
There was a problem hiding this comment.
In general, what if we allow the assessment workflow to receive the completed mocked object to be returned by the worker?
For instance, if load-assessment-consolidated-data is supposed to return:
{
"someVariable1": 123,
"someVariable2": 444,
"someVariableN": 555,
}We can start the workflow with the payload as:
{
"assessmentId": 1,
"e2eLoadAssessmentConsolidatedDataResult": {
"someVariable1": 123,
"someVariable2": 444,
"someVariableN": 555,
}
}The work from the worker method would be just to return the workflow variable e2eLoadAssessmentConsolidatedDataResult, thoughts?
In this example, we can still use the getMockConsolidatedData to support the data creation, but this would be part of the Arrange area of each test as needed.
The only point will be how we can handle mocks for jobs that are reused, for instance, income verification for the student and his parents. In this scenario, the job probably has some information about the id of the invoked worker task that can be used to differentiate one from another.
| }); | ||
| // Workflow instance expected to pass through associate workflow instance worker. | ||
| expect( | ||
| assessmentGatewayResponse.variables[Workers.AssociateWorkflowInstance], |
There was a problem hiding this comment.
Some workers are reused and then have the same task name, for instance, update-application-status can be called multiple times during the workflow execution. We can maybe use the service tasks id (also available on worker jobs object) to execute the asserts.
There are other cases where, for instance, a worker like create-income-request-task can be invoked by 1 to 3 tasks (Student+Parent1+Parent2). For this scenario, we may need to add something to the worker header, which will be a minor bpmn change, but with a great benefit IMO.
| // as this is a reassessment. | ||
| expect( | ||
| assessmentGatewayResponse.variables[Workers.UpdateApplicationStatus], | ||
| ).toBeUndefined(); |
| import { Duration, ZBClient, ZBWorkerConfig } from "zeebe-node"; | ||
| import { ApplicationExceptionStatus, ProgramInfoStatus } from "@sims/sims-db"; | ||
| import { Workers } from "../constants/worker-constants"; | ||
|
|
There was a problem hiding this comment.
mm, I need a walk-through of this file logic
sources/packages/backend/workflow/test/test-utils/mock/zeebe-worker-mock.ts
Outdated
Show resolved
Hide resolved
...workflow/test/2021-2022/assessment-gateway/assessment-gateway.originalAssessment.e2e-spec.ts
Show resolved
Hide resolved
...workflow/test/2021-2022/assessment-gateway/assessment-gateway.originalAssessment.e2e-spec.ts
Outdated
Show resolved
Hide resolved
...kend/workflow/test/2021-2022/assessment-gateway/assessment-gateway.studentAppeal.e2e-spec.ts
Outdated
Show resolved
Hide resolved
...workflow/test/2022-2023/assessment-gateway/assessment-gateway.originalAssessment.e2e-spec.ts
Outdated
Show resolved
Hide resolved
| @@ -0,0 +1,18 @@ | |||
| export enum WorkflowServiceTasks { | |||
sources/packages/backend/workflow/test/test-utils/factories/assessment-consolidated-data.ts
Show resolved
Hide resolved
sources/packages/backend/workflow/test/test-utils/mock/zeebe-worker-mock.ts
Outdated
Show resolved
Hide resolved
| }, | ||
| requestTimeout: PROCESS_INSTANCE_CREATE_TIMEOUT, | ||
| }); | ||
| expectToPassthroughServiceTasks( |
There was a problem hiding this comment.
expectToPassThroughServiceTasks?
| * @param workflowResultVariables workflow result variables | ||
| * @param serviceTasks service tasks to verify. | ||
| */ | ||
| export function expectNotToPassthroughServiceTasks( |
There was a problem hiding this comment.
caps expectNotToPassThroughServiceTasks
...kend/workflow/test/2021-2022/assessment-gateway/assessment-gateway.studentAppeal.e2e-spec.ts
Outdated
Show resolved
Hide resolved
| assessmentTriggerType: AssessmentTriggerType.OriginalAssessment, | ||
| ...createFakeSingleIndependentStudentData(), | ||
| // Application with no exception. | ||
| applicationExceptionStatus: ApplicationExceptionStatus.Approved, |
There was a problem hiding this comment.
Is this data not expected to be mocked on verify-application-exceptions-task-result?
| @@ -5,7 +5,7 @@ services: | |||
| environment: | |||
| - ZEEBE_BROKER_DATA_DISKUSAGECOMMANDWATERMARK=0.998 | |||
| - ZEEBE_BROKER_DATA_DISKUSAGEREPLICATIONWATERMARK=0.999 | |||
| - "JAVA_TOOL_OPTIONS=-Xms256m -Xmx256m" | |||
| - "JAVA_TOOL_OPTIONS=-Xms512m -Xmx512m" | |||
There was a problem hiding this comment.
In the other PR I would comment on that. I don't know how it was possible to run that with only 256m 😄
|
Kudos, SonarCloud Quality Gate passed!
|
Backend Unit Tests Coverage Report
|
E2E Queue Consumers Coverage Report
|
E2E Workflow Workers Coverage Report
|
E2E SIMS API Coverage Report
|
E2E Tests for assessment gateway
E2E Tests for assessment gateway
E2E Tests for Assessment Gateway
Assessment Gateway testing strategy
The assessment gateway workflow's primary responsibility is to obtain the necessary details of a student assessment w.r.t it's instance and validate the details to ensure the business process is followed and through a sequence of steps.
Ignoring the process of obtaining and updating the data, we are going to validate the orchestration of assessment gateway in the E2E Tests as per the student assessment and application details provided to the workflow.
E2E Scenarios tested
Both the above scenarios are tested for all given program years.
How do we validate the workflow output
Assessment gateway output on Original assessment:
The highlighted blocks are verified in the e2e tests.
Assessment gateway output on Student appeal:
The highlighted blocks are verified in the e2e tests(the ones which are crossed out is verified as NOT to be passed through).