Skip to content
This repository has been archived by the owner on Dec 22, 2022. It is now read-only.

Use vcr for external services #1464

Merged
merged 2 commits into from
Jan 17, 2019
Merged

Use vcr for external services #1464

merged 2 commits into from
Jan 17, 2019

Conversation

pgwillia
Copy link
Member

closes #1463

Testing is failing because the record we use to test the electronic holdings no longer has any holdings. Falsified the marc xml to use a more current id of this item. Added vcr to record use of external services.

Testing is failing because the record we use to test the electronic 
holdings no longer has any holdings.
@pgwillia
Copy link
Member Author

@seanluyk do you know if there is anything in the ws terms of service that prevents us from recording/storing fixtures/vcr_cassettes/?

@pgwillia pgwillia requested a review from murny January 17, 2019 21:43
@@ -6,6 +6,8 @@
require 'active_support/inflector'
require 'capybara/rspec'
require 'capybara/rails'
require 'vcr'
Copy link
Contributor

@murny murny Jan 17, 2019

Choose a reason for hiding this comment

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

No idea why we have this file?

The point of spec_helper vs rails_helper is spec_helper is suppose to be a lightweight version that doesn't need or depend on all the rails stuff. But here we bloat everything inside this blacklight_helper, then include it in spec_helper (where spec_helper shouldn't need all this stuff like capybara/rails/etc)...which is then included in rails_helper.

No change required here. More so just documenting this and linking this comment to this our testing refactor issue so we can tackle this later

#1330

Copy link
Contributor

@murny murny left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Nice work!

@pgwillia pgwillia merged commit d9f807e into ualbertalib:master Jan 17, 2019
@pgwillia pgwillia deleted the 1463_vcr_for_external_services branch January 17, 2019 22:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use VCR for tests that hit external services
2 participants