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 RSpec unit testing when using Docker dev setup #10950
Fix RSpec unit testing when using Docker dev setup #10950
Conversation
34d67b2
to
1eb0308
Compare
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.
Thanks for the fix! Since this does indeed solve the ticket I will approve, but I made a suggestion for also checking Redis if you have the time.
Thanks for your contribution!
spec/rails_helper.rb
Outdated
| @@ -48,6 +48,7 @@ | |||
| "selenium-release.storage.googleapis.com", | |||
| "developer.microsoft.com/en-us/microsoft-edge/tools/webdriver", | |||
| "api.knapsackpro.com", | |||
| "elasticsearch:9200", | |||
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.
We are in the process of setting up Docker for our CI and this is one of the things I also had to add so +1 for doing it here. I think you might also need to add redis here as well. Worth checking.
Also, I believe you only need the host, elasticsearch and not the port.
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.
Also, I believe you only need the host,
elasticsearchand not the port.
True, I added the port to be more specific but in this case it shouldn't be an issue.
I think you might also need to add redis here as well. Worth checking.
It's possible I missed it because I get some other unrelated failures such as missing Chrome binary. However I ran the entire RSpec suite with my Docker dev setup and I didn't see a VCR or WebMock failure for Redis requests, so I'm not sure it's needed. At least not until someone explicitly makes some Redis requests from RSpec.
e07d671
to
769d1ec
Compare
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.
LGTM!
What type of PR is this? (check all applicable)
Description
When using the Docker dev setup, Elasticsearch is available at
elasticsearch:9200instead oflocalhost:9200so the VCR testing config was not ignoring the requests needed for RSpec setup when doingSearch::Cluster.recreate_indexesinspec/rails_helper.rb. Also the WebMock setup allowslocalhostrequests which should also include Elasticsearch requests.This seems okay to me, but I'm not sure if there is a better solution. One issue I might have is that a detail related to the Docker dev setup is bleeding into common RSpec configs. What do you think, @citizen428?
Related Tickets & Documents
Closes #10579
QA Instructions, Screenshots, Recordings
With this change you can run unit tests when using the Docker dev setup, for example this spec was failing at
Search::Cluster.recreate_indexesinspec/rails_helper.rband is now passing.Added tests?
Added to documentation?