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

Fix "search with a filter with arguments" spec #2581

Merged
merged 1 commit into from
May 14, 2024

Conversation

CaleCrawford
Copy link
Contributor

Hi. This PR will solve the failing flakey test listed in #2523. While debugging the issue I found a few things.

The flake would happen when the second search of "kind:vip" was not applied to the url. I was able to run the test in headed chrome and see the failure happen. The url param was never set and was therefore blank. Adding a sleep(0.1) after the clear_search seemed to work as one pathway. Capybara

I went down the path of modifying the existing test to pass with a single param by the suggestions of a few at the RailsConf hackathon. Happy to add an additional scenario testing "kind:standard" if that would be preferred.

Thanks!

Copy link
Contributor

@littleforest littleforest left a comment

Choose a reason for hiding this comment

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

This looks great, thanks for fixing! I don't think the second filter test is actually necessary. Would you mind fixing the linting?

@CaleCrawford
Copy link
Contributor Author

@littleforest sounds good thank you! Should be fixed now

The flake would happen when the second search of `kind:vip` was not
applied to the URL. Running the test directly in Chrome would show it
happening. As the URL parameter was never set, it was therefore blank.
Whilst a `sleep(0.1)` would fix it, this instead restructures the test
to avoid the problem completely.

Closes thoughtbot#2523
@nickcharlton
Copy link
Member

Wonderful! Thank you so much for taking the time to look at this, it's been a real thorn-in-my-side for the last weeks, and I'm terrible when it comes to flakey specs!

I'm really glad the RailsConf hackathon turned out so well, too!

@nickcharlton nickcharlton merged commit d70f37e into thoughtbot:main May 14, 2024
10 checks passed
@CaleCrawford
Copy link
Contributor Author

@nickcharlton glad it worked out! haha I hear you, same. Yeah really enjoyed the hackathon.

I let Steffani know that I run the open source guild at my company and we have a few people who are looking for places to contribute. Would definitely love to stay in contact if you ever need a few contributors to take on similar things

@nickcharlton
Copy link
Member

Oh! That's cool to hear! We've started catching up as maintainers internally, so we'll have a chat about how we can make it a bit clearer on how to jump in. In the case of Administrate, it's really just picking something that interests you — but I appreciate that can be quite hard of a suggestion!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants