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
test(quantic): SFINT-5832 Sort E2E tests migrate from Cypress to Playwright #4777
base: master
Are you sure you want to change the base?
test(quantic): SFINT-5832 Sort E2E tests migrate from Cypress to Playwright #4777
Changes from 1 commit
80483a0
9abd2b7
de96a13
e365d55
45934bc
a480bcf
46d133e
1264be9
3a8ae43
80cfcb5
7b00d6a
8a6676a
1d41583
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 my opinion for your test to be robust, you shouldn't rely on "Newest" to be the test option.
Your test should be a bit more agnostic to which Sort options exist.
Example
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.
no, for e2e test small happy path like this, I don't think we need it too robust, as we also want to be sure the order of 'sort' is the same as what we expect at this moment
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.
That's just the thing, I see it as the opposite, if the order of sort changes, we don't want the test to fail.
We're testing that the sort you selected is what actually gets sent in UA and in the search response.
The order doesn't matter, it could be the 2nd, 3rd, whatever. It just needs to be the same.
It is what we do in our other tests, so please make that small change so our test doesn't break if the ordering of the sort changes. :)
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.
Again here, I would go fetch what the loaded sorts are available on the page instead of relying on the default sorts.
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.
same as above, we want to be sure the sort displays in correct order
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.
Order of sorts is not in the contract of our component. It's a parameter we receive and then display. We have unit tests to check that the sort are displayed in the order we've received them.
The e2e test shouldn't rely on the order of sort options to pass or fail. Please make this small change :)
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 does the "focusSortDropDownEnter" mean here?
The function clicks on the preview button and then presses Tab?
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 name of this function doesn't describe what is being done with the steps within this function, what are you trying to accomplish with that?