(test) Use UI flow instead of direct API calls to test removing a patient from a list #1487
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
NethmiRodrigo
force-pushed
the
test/update-delete-patient-from-list
branch
from
281a371 to
2f1b186
Compare
|
Size Change: 0 B Total Size: 6.27 MB ℹ️ View Unchanged
|
NethmiRodrigo
force-pushed
the
test/update-delete-patient-from-list
branch
from
2f1b186 to
7762148
Compare
vasharma05
reviewed
Feb 17, 2025
|
Another day, another "why does this e2e test pass locally and fail here????" |
denniskigen
reviewed
Feb 17, 2025
denniskigen
reviewed
Feb 17, 2025
4 tasks
|
The problem here is that the patient lists header count is supposed to update and show |
|
To add more context, this PR tries to update the e2e test that deletes a patient from a list by using the user interface. Up until now, this test did this by having a function to directly call the API. In these two PRs - openmrs/openmrs-esm-core#1280, openmrs/openmrs-esm-core#1290, this test started failing because the API call failed for some reason. Which is why I tried to resort to using the UI, because we can and we should anyway. |
denniskigen
force-pushed
the
test/update-delete-patient-from-list
branch
from
5df3522 to
3a67a53
Compare
|
@jayasanka-sack would appreciate your input on this |
|
The issue could be related to time zones (client vs backend); @NethmiRodrigo try voiding the membership explicitly here: return postData(`${cohortUrl}/cohortmember/${cohortMembershipUuid}`, {
endDate: new Date(),
voided: true
}); |
@samuelmale sorry I don't understand. Since the delete button calls this function on click, doesn't this already happen? |
denniskigen
approved these changes
Feb 19, 2025
|
Oh right, yeah I completely forgot that we removed that. Do you think it's better to just close this PR @denniskigen |
|
I think we can merge it in and sort things out when we restore the two modules in the future. |
Previously, the patient list e2e test was using direct API calls to test patient removal. This change: - Breaks down the single test step into multiple, more specific steps - Tests the actual user flow by interacting with the UI (clicking remove button, confirming in modal) - Adds verification of success message and patient count updates - Adds appropriate timeout for patient count update This makes the test more valuable by verifying the complete user experience rather than just the API integration.
denniskigen
force-pushed
the
test/update-delete-patient-from-list
branch
from
bde48d3 to
ea8cd8f
Compare
denniskigen
changed the title
|
Oh wait, both frontend modules still exist in the e2e test docker environment. Maybe we should explicitly omit them from inclusion in |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Requirements
Summary
The e2e test for managing a patient list heavily relies on directly making API calls rather than interacting with the user interface, which defeats the purpose of it existing. This updates just the delete patient to hopefully get more info as to why it fails in this PR.
Screenshots
Related Issue
Other