Skip to content
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

Frontend Tests: Standardize userEvent usage to conform with upstream recomendations #5872

Merged
merged 3 commits into from
Jun 22, 2023

Conversation

tsmock
Copy link
Contributor

@tsmock tsmock commented Jun 2, 2023

userEvent recommends using userEvent.setup instead of userEvent.click (or other global methods).

This also replaces most usages of fireEvent with userEvent equivalents.

@santosh-kuchetti

This comment was marked as off-topic.

@HelNershingThapa

This comment was marked as off-topic.

@santosh-kuchetti

This comment was marked as off-topic.

@HelNershingThapa

This comment was marked as off-topic.

@santosh-kuchetti

This comment was marked as off-topic.

@tsmock

This comment was marked as off-topic.

@HelNershingThapa
Copy link
Contributor

@tsmock At this commit: 7e3e2df, I made some further modifications. I discovered additional locations where we might standardize the use of userEvent and included any needed awaiting the same. Could you look over and sync?

@tsmock tsmock force-pushed the chore/userEvent-cleanup branch 2 times, most recently from 04d2f89 to a4e15ee Compare June 8, 2023 12:06
@tsmock
Copy link
Contributor Author

tsmock commented Jun 8, 2023

It should be good now; I'm just waiting for tests to run.

tsmock and others added 3 commits June 8, 2023 07:26
For details, see https://testing-library.com/docs/user-event/setup .
Existing uses call `userEvent.setup()` internally in order to ease
migration to v14. The compatibility call will most likely be removed in
a future version.

This also moves most tests to userEvent from fireEvent.

Signed-off-by: Taylor Smock <tsmock@meta.com>
Signed-off-by: Taylor Smock <tsmock@meta.com>
@tsmock
Copy link
Contributor Author

tsmock commented Jun 8, 2023

Sanity check: I'm getting randomly failing tests again. Can you reproduce that on your end? If so, I think we accidentally found the blocker for #5721.

Note: I've found that the tests fail more reliably if the system is under load.

@sonarcloud
Copy link

sonarcloud bot commented Jun 8, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.1% 0.1% Duplication

@HelNershingThapa
Copy link
Contributor

I have these 2 test cases failing on my end.

 FAIL  src/views/tests/taskSelection.test.js (67.315 s)
   Task Selection Page  should change the button text to map selected task when user selects a task

    thrown: "Exceeded timeout of 5000 ms for a test.
    Use jest.setTimeout(newTimeout) to increase the timeout value, if this is a long-running test."

      90 |   });
      91 |
    > 92 |   it('should change the button text to map selected task when user selects a task', async () => {
         |   ^
      93 |     act(() => {
      94 |       store.dispatch({
      95 |         type: 'SET_USER_DETAILS',

      at src/views/tests/taskSelection.test.js:92:3
      at Object.<anonymous> (src/views/tests/taskSelection.test.js:12:1)

   Task Selection Page  should change the button text to validate selected task

    thrown: "Exceeded timeout of 5000 ms for a test.
    Use jest.setTimeout(newTimeout) to increase the timeout value, if this is a long-running test."

      150 |   });
      151 |
    > 152 |   it('should change the button text to validate selected task', async () => {
          |   ^
      153 |     act(() => {
      154 |       store.dispatch({
      155 |         type: 'SET_USER_DETAILS',

      at src/views/tests/taskSelection.test.js:152:3
      at Object.<anonymous> (src/views/tests/taskSelection.test.js:12:1)

@tsmock
Copy link
Contributor Author

tsmock commented Jun 8, 2023

When I was troubleshooting #5721, I kept a log of what tests had failed. Both of the tests that you had fail failed for me (but not on a regular basis; out of 10 runs, they failed once).

Test runs from #5721
File Test RPi (Node 16, develop) Mac (Node 16, updates) Mac (Node 18, updates)
src/components/projectCard/tests/dueDateBox.test.js test DueDate › with tooltip message     1
src/components/projectDetail/tests/questionsAndComments.test.js test if QuestionsAndComments component › renders tabs for writing and previewing comments   1 1
src/components/projectDetail/tests/shareButton.test.js test if shareButton › render open corresponding window popup     1
src/components/projectDetail/tests/shareButton.test.js test if shareButton › render shareButton for project with id 1     1
src/components/taskSelection/tests/actionSidebars.test.js Appearance of unsaved map changes to be dealt with while mapping › when splitting a task   2 1
src/components/taskSelection/tests/actionSidebars.test.js Appearance of unsaved map changes to be dealt with while mapping › when submitting a task     1
src/components/taskSelection/tests/actionSidebars.test.js Completion Tab for Validation › should update status and comments for multiple tasks   1  
src/components/taskSelection/tests/contributions.test.js Contributions › clean user selection if we click on the selected tasks of the user 1    
src/components/taskSelection/tests/contributions.test.js Contributions › render users, links, and 1 1 1
src/components/taskSelection/tests/index.test.js Contributions › should clear text when close icon is clicked 3    
src/components/taskSelection/tests/index.test.js Contributions › should select tasks mapped by the selected user 10    
src/components/taskSelection/tests/index.test.js Contributions › should select tasks validated by the selected user 10    
src/components/taskSelection/tests/index.test.js Contributions › should sort tasks by their task number 10    
src/components/taskSelection/tests/taskActivity.test.js TaskHistory › renders the task history comments and activities for a given project   1 1
src/components/taskSelection/tests/taskList.test.js Task Item › should display task detail modal   1 1
src/components/tests/selectAll.test.js SelectAll › unactive when clicked select all elements     1
src/views/tests/interests.test.js Edit Interest › should hide the save button on click     1
src/views/tests/interests.test.js Edit Interest › should return input text value to default when cancel button is clicked     1
src/views/tests/management.test.js Management Page Overview Section › should list projects and teams fetched 2    
src/views/tests/project.test.js CreateProject renders ProjectCreate   1  
src/views/tests/project.test.js Project Detail Page › should render component details 10 1  
src/views/tests/projectStats.test.js ProjectStats dashboard › fetch urls and render sections title   1 2
src/views/tests/taskSelection.test.js Random Task Selection › should not change the button text to map selected task when user selects a task for mapping 10    
src/views/tests/taskSelection.test.js Task Selection Page › should change the button text to map another task when user selects a task for validation but the user level is not met 7    
src/views/tests/taskSelection.test.js Task Selection Page › should change the button text to map selected task when user selects a task 10 1 1
src/views/tests/taskSelection.test.js Task Selection Page › should change the button text to validate selected task 10 1 1
src/views/tests/taskSelection.test.js Task Selection Page › should filter the task list by search query 8 2 1
src/views/tests/teams.test.js Edit Team › should display default details of the team before editing     2
src/views/tests/userDetail.test.js User Detail Component › should render detail for the child components   1 1

@HelNershingThapa
Copy link
Contributor

Did all the test cases that failed in the log mention the same error message Exceeded timeout of 5000 ms for a test?

@tsmock
Copy link
Contributor Author

tsmock commented Jun 8, 2023

No; some of them were due to not finding a specified element within a specified amount of time (1s, IIRC, but I ended up testing with 5s).

EDIT: The hard part is that sometimes I'll have a string of passing test runs, and then a failing test run with multiple test failures. It makes it a PITA to debug.

@HelNershingThapa
Copy link
Contributor

Ran the test cases 5 times. I'm on Ubuntu with Node v16.

Test Case Failures
Task Selection Page › should change the button text to map selected task when user selects a task 3
Task Selection Page › should change the button text to validate selected task 4
Task Selection Page › should filter the task list by search query 2
Task Selection Page › should change the button text to map another task when user selects a task for validation but the user level is not met 2

@HelNershingThapa
Copy link
Contributor

yeah it's been a strange scenario. And when only one file is examined, these scenarios would always pass. So strange. I ran tests on the /src/views/tests/taskSelection.test.js/ file numerous times and they always pass.

@tsmock
Copy link
Contributor Author

tsmock commented Jun 8, 2023

Yes. Trying to debug the failures has been aggravating. Sometimes I'll change something, get a few runs in with no tests failing, think I've fixed the problem, then the next run fails.

It doesn't help that my IDE doesn't have a "test until failure option".

@HelNershingThapa HelNershingThapa merged commit c097ed8 into hotosm:develop Jun 22, 2023
3 checks passed
@tsmock tsmock deleted the chore/userEvent-cleanup branch June 22, 2023 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants