Skip to content
This repository was archived by the owner on Nov 23, 2024. It is now read-only.

Use VCR for tests #42

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

Use VCR for tests #42

wants to merge 14 commits into from

Conversation

petems
Copy link
Contributor

@petems petems commented Feb 10, 2014

I've got a big-refactor on my branch that uses faraday instead of oauth, not sure if you guys are up for that, but I realised I would have no way of easily proving that it worked.

So, I enabled VCR for this project! The recordings were made to a dummy Dropbox that I don't care about, so there shouldn't be any issues with data leakage (I did filter the headers though, in case things need to be re-recorded! 😄)

@marcinbunsch any thoughts? 👍

@petems
Copy link
Contributor Author

petems commented Feb 10, 2014

Another cool thing is:

Coverage report generated for RSpec to /Users/peterso/Projects/dropbox-api/coverage. 287 / 287 LOC (100.0%) covered.

100% LOC coverage 💃

@petems
Copy link
Contributor Author

petems commented Aug 17, 2014

@marcinbunsch Can I get a merge? 👍

@marcinbunsch
Copy link

I finally found time to play around with. The work you've done is impressive. Here's the issue - it will only work on your dummy account, which is prepopulated with files. When I run it on my empty account, I get 14 failures.

I think we should rethink the strategy here. Let's assume that the account starts empty. So we need to populate before testing and we need to avoid testing of account-specific things like the email or tokens. In the end it's irrelevant who's account we run it on, it will work properly. Let me know if you want to work on it, if not, I can take it from here.

@petems
Copy link
Contributor Author

petems commented Oct 30, 2014

Hmm, we could have a fixture to say "if recording for VCR, then do these pre-populate steps" or something. Then it would work regardless of whether it's a dummy account or not. I don't mind pairing on it 👍

Let me rebase to fix the merge conflicts also.

I created a dummy user that I can delete afterwards just in case I leak any data, but I've added the authorisation to the filtered config so if they're re-recorded in the future they'll be no issues
@marcinbunsch
Copy link

@petems I've added you to a group which gives you write access to the repo. One, you've done great work 👍 Two, it will make it easier to pair and bring this PR home.

Feel free to add this as a branch within futuresimple/dropbox-api.

@petems
Copy link
Contributor Author

petems commented Nov 4, 2014

@marcinbunsch Awesome thanks! I'm gonna rebase this with a basic version of recording the responses in a smaller chunk so it can be merged, then we can pair on the implementation after 👍

@petems petems mentioned this pull request Dec 3, 2014
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.

2 participants