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

Flaky test issue 5424 #5439

Merged
merged 7 commits into from
Jan 15, 2024
Merged

Flaky test issue 5424 #5439

merged 7 commits into from
Jan 15, 2024

Conversation

hroulston
Copy link
Collaborator

What github issue is this PR for, if any?

Resolves #5424

What changed, and why?

In case_contact_report.rb file I ordered the Case Contacts by id under filtered_case_contacts method. Test passes since the order will match what is expected.

How will this affect user permissions?

  • Volunteer permissions: n/a
  • Supervisor permissions: n/a
  • Admin permissions: n/a

How is this tested? (please write tests!) 💖💪

Test already in place, didn't need to change anything.

Screenshots please :)

Screenshot 2023-12-12 at 1 34 13 PM

Feelings gif (optional)

What gif best describes your feeling working on this issue? https://giphy.com/
How to embed:
![alt text](https://media.giphy.com/media/1nP7ThJFes5pgXKUNf/giphy.gif)

Feedback please? (optional)

We are very interested in your feedback! Please give us some :) https://forms.gle/1D5ACNgTs2u9gSdh9

@github-actions github-actions bot added dependencies Pull requests that update a dependency file ruby Pull requests that update Ruby code labels Dec 12, 2023
@@ -25,6 +25,7 @@ def filtered_case_contacts(args)
.contact_type(args[:contact_type_ids])
.contact_type_groups(args[:contact_type_group_ids])
.with_casa_case(args[:casa_case_ids])
.order(:id)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you change the test instead of the app code?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure no problem. Can I ask why it is preferable to go with Active Record's flaky ordering instead of explicit ordering? Happy to make the change, just pure curiosity.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally you don't wanna change the app code to make a test run. You want to just change the test.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah got it, thank you :)

@hroulston hroulston force-pushed the flaky_test_issue_5424 branch from e89aaba to 29d8182 Compare December 14, 2023 04:13
@hroulston hroulston force-pushed the flaky_test_issue_5424 branch from 29d8182 to cffebc2 Compare December 14, 2023 04:15
@hroulston hroulston requested a review from FireLemons December 14, 2023 04:16
Copy link
Collaborator

@schoork schoork left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reverted the Gemfile.lock commit, since it didn't have to do with the test.

I like this match_array matcher. I had to do some reading on that. Nice work @hroulston!

@schoork schoork enabled auto-merge January 15, 2024 20:43
@schoork schoork disabled auto-merge January 15, 2024 21:22
@schoork schoork merged commit 54790c8 into main Jan 15, 2024
12 checks passed
@schoork schoork deleted the flaky_test_issue_5424 branch January 15, 2024 21:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file ruby Pull requests that update Ruby code Tests! 🎉💖👏
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: Flaky Test Case Contact Filter Mismatch
5 participants