LyricsGenius
LyricsGenius copied to clipboard
Added vcr support for consistent tests
Only tests functions that use HTTP will have the vcrpy decorator
Hi @ludehon, sorry for the delay. Do you mind adding a demonstration of how to run these VCR tests?
Reading the Rationale on VCR's docs, I think this might be a good idea. Currently, the tests take about 3:20 minutes on Travis CI and even longer on local machines.
Using VCR without the libyaml extension the tests took 105 seconds on my PC. It's also nice to be able to run the tests offline.
Do you mind addressing these merge conflicts, @Allerter?
@johnwmillr, not at all. I had actually tried this out a while ago. So this won't take much time.
A few notes:
- The format of the cassettes can be changed to JSON when initiating
test_vcr
by changingserializer='yaml'
. But for some of the tests, I explicitly set the serializer to YAML because those tests directly or indirectly callgenius.lyrics
orgenius.tag
which the response of its request is binary and JSON can't normally encode that (unless we make our own serializer forvcrpy
.) - These cassettes only work from the second time tests are run, since at the first run
vcrpy
records the cassettes. So Travis CI won't be using recorded cassettes. This won't reduce test time on Travis CI but has two advantages: 1. It's best that Travis CI test all endpoints live to check if an endpoint has become unavailable. 2. Putting the recorded cassettes will increase the size of the repository, especially more since we're downloading whole HTML pages when usinggenius.lyrics
andgenius.tag
. - Although the recorded cassettes won't be in the repo, I still added filtering the
authorization
headers to avoid revealing them even in local machines.
- [x] Ready to be merged
@johnwmillr if you think using vcrpy
isn't worth the trouble, you can close this pull request. It's not necessary and it's only useful when testing locally.
You're right, @Allerter. For the sake of moving forward with 3.0, let's hold off on this PR for now. I won't close it just yet, but we can move forward with the 3.0 work without merging this in.