-
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
#1834 - Hierarchical mocks for workflow E2E tests #1865
Conversation
return createMockedWorkerResult( | ||
WorkflowServiceTasks.LoadAssessmentConsolidatedData, | ||
{ | ||
subprocess: |
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.
pls add comments and review the 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.
Comments updated.
* - `result`: suffix that identifies the job completed object. | ||
* @param serviceTaskId service task id that will have the job completed returned. | ||
* @returns mock identifier for a completed job to be sent to the workflow. | ||
* - `passthrough`: suffix that identifies the job passthrough identifier. |
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.
passthrough
is just a word change right, logically it is the same right. it represent the passed/completed task/flow
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.
Yes, I was not so sure at the beginning but it seems to be "The act or process of passing through" (https://en.wiktionary.org/wiki/passthrough#). If it is confusing I can change it.
sources/packages/backend/workflow/BPMN/camunda-8/load-assessment-consolidated-data.bpmn
Outdated
Show resolved
Hide resolved
@@ -66,7 +56,9 @@ export class ZeebeMockedClient { | |||
taskType, | |||
taskHandler: mockTaskHandler, | |||
})); | |||
ZeebeMockedClient.mockedZeebeClient = new ZBClient(); | |||
// Zeebe client logs disabled for a better test summary display. | |||
// It can be enable as needed for troubleshooting. |
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.
👍
sources/packages/backend/workflow/BPMN/camunda-8/assessment-gateway.bpmn
Show resolved
Hide resolved
...workflow/test/2022-2023/assessment-gateway/assessment-gateway.originalAssessment.e2e-spec.ts
Outdated
Show resolved
Hide resolved
const dataPreAssessment: AssessmentConsolidatedData = { | ||
assessmentTriggerType: AssessmentTriggerType.OriginalAssessment, | ||
...createFakeConsolidatedFulltimeData(PROGRAM_YEAR), | ||
...createParentsData({ |
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.
Good effort @andrewsignori-aot 👍
@@ -28,15 +31,19 @@ describe(`E2E Test Workflow assessment gateway on student appeal for ${PROGRAM_Y | |||
...createFakeSingleIndependentStudentData(), | |||
}; | |||
|
|||
const workersMockedData = createWorkersMockedData([ | |||
createLoadAssessmentDataTaskMock({ | |||
assessmentConsolidatedData: assessmentConsolidatedData, |
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.
We are ignoring the subprocess context here, which is great 👍
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.
Yes, I resolved this comment using the same 😉
#1865 (comment)
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.
/** | ||
* Suffix that indicates if a task or subprocess was invoked by the workflow. | ||
*/ | ||
export const JOB_PASSTHROUGH_SUFFIX = "passthrough"; |
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.
What was the intention having passthrough suffixed(in the bpmn output value of variable)? to follow the pattern of aligning with it's purpose?
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.
Without the suffix, we would be actually updating the entire mocked object that is now the root of the mock.
For instance, we would be replacing the below:
"verify_application_exceptions_task": {
"result": {
"applicationExceptionStatus": "Approved"
}
},
by
"verify_application_exceptions_task": "verify_application_exceptions_task"
Kudos, SonarCloud Quality Gate passed!
|
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, Great work again 👍
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, good work @andrewsignori-aot
The service task ids are now the root of the mocked objects. When the same service task id exists for multiple subprocesses, these subprocesses are added as a child of the root service task id object. In the below sample,
create_supporting_users_for_parents_task
is not invoked by any subprocess, so the mocked objects forresult
andmessageResult
are direct properties of the root service task id. On the other hand,create_income_request_task
is invoked by multiple subprocesses, andresult
andmessageResult
are associated with each one of them.The
load-assessment-data-?
subprocess, which is potentially invoked twice, is now treated as a regular subprocess where we should provide mocked data for both subprocesses (the same data can be provided twice).Sample Hierarchical Mocks
Please that some
null
or empty values were removed from consolidated data for better visualization here.The main differences between
load_assessment_data_submit_or_reassessment_subprocess
andload_assessment_data_pre_assessment_subprocess
are related to the variables prefixed withparent1
andparent2
.Resuing the same mock for multiple subprocesses.
A mocked object can be reused for multiple tasks under different subprocesses, as shown below.
Test summary readability
To allow better readability of the test summary, the Zeebe logging is disabled.
Varibales naming conventions
The possible change in the naming conventions for the service task ids is not considered for this PR.
WorkflowServiceTasks
andWorkflowSubprocesses
are now following the same convention, as shown below.Right now service task ids are still using the "-" as separators while Camunda E2E-related variables are using "_", which makes it easier to identify a variable on the workflow that is related to E2E.