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

Fixes rspec tests that fail with a clean test db #112

Merged
merged 2 commits into from
Jul 20, 2018
Merged

Fixes rspec tests that fail with a clean test db #112

merged 2 commits into from
Jul 20, 2018

Conversation

HalahAb
Copy link
Contributor

@HalahAb HalahAb commented Jul 18, 2018

No description provided.

@HalahAb
Copy link
Contributor Author

HalahAb commented Jul 18, 2018

Resolves #59

@@ -17,6 +17,9 @@ def parse_search_engine_response(_)
end

describe SearchEngine do
before(:all) do
CachedSearchApiConnectionResponse = Struct.new(:response, :cache_namespace)
Copy link
Contributor

Choose a reason for hiding this comment

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

@HalahAb , I see the issue with this spec, but I don't think that redefining the struct in the spec is the best approach. I think a safer fix would be to move the struct definition to lib/cached_search_api_connection_response.rb, to ensure that that class is auto-loaded.

@@ -0,0 +1,3 @@
# :nocov:
Copy link
Contributor

Choose a reason for hiding this comment

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

@HalahAb , please add a spec for this instead of skipping code coverage.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@MothOnMars, I added a test then had to remove it because that one line was still being seen as missing coverage. I couldn't figure out why and thought to remove it because it was only testing ruby struct functionality. I added the test back now, but it turns out it was the other struct definition in cached_search_api_connection.rb that was causing the coverage fail. I removed it and that seems to have fixed things.

require 'spec_helper'

describe CachedSearchApiConnectionResponse do
let(:cached_response) { CachedSearchApiConnectionResponse.new(:response, :cache_name) }
Copy link
Contributor

Choose a reason for hiding this comment

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

please remove extra space: { CachedSearchApiConnectionResponse...

before do
allow(affiliate).to receive(:search_engine).and_return(search_engine)
allow(affiliate).to receive(:has_no_social_image_feeds?).and_return false
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove extra space: and_return false

@MothOnMars MothOnMars merged commit 2ac8d6b into GSA:master Jul 20, 2018
@MothOnMars MothOnMars deleted the fix_rspec_test_fail_with_clean_db branch July 20, 2018 23:29
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.

2 participants