Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
#1722 - E2E Tests for assessment gateway #1822
#1722 - E2E Tests for assessment gateway #1822
Changes from 1 commit
82eb9f5
fbaefe5
cb0b09f
64e1f5b
329edd3
6c18373
624148b
9924a9c
1eb1989
792429a
91fef6b
81f12b1
0bd58d7
271c858
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
This file was deleted.
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, there is nothing against creating a new
createFakeAssessmentConsolidatedData
with some initial data as a baseline, for instance.createFakeAssessmentConsolidatedDataForTwoParents
createFakeAssessmentConsolidatedDataWithDependents
createFakeAssessmentConsolidatedDataWithPartner
createFakeAssessmentConsolidatedDataForPIR
Or have all of them as options on the
createFakeAssessmentConsolidatedData
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.
mm, I need a walk-through of this file logic
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.
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:We can start the workflow with the payload as:
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 theArrange
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.
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the other PR I would comment on that. I don't know how it was possible to run that with only 256m 😄