Integration tests

[jjb]

I think the project would benefit from integration tests. Bugs that PRs like #35, #89, and #97 would be easier to observe with integration tests.

Here is some initial simple code for the purpose of discussion. Things to consider:

• in general do we want this?
• I've put all the existing tests into "unit" — does that seem appropriate?
• currently there is no HTTP mocking framework and a real request is made to facebook. what mocking framework do we like? the two I have used are fakeweb and VCR. fakeweb is simpler and takes less DSL syntax to apply the mocking to the entire suite. in the absense of other factors, I would use fakeweb. VCR is more featureful and more actively developed. perhaps others have more experience with these or other projects and are more opinionated.

[mislav]

We do integration tests in Faraday and they are very useful. Yes I think we should have a stronger test suite in the light of these bugs.

I would use webmock instead of fakeweb (outdated) or VCR (overkill for our purposes).

[jjb]

okay, now mocked with webmock

i could make a spec helper file for the integration tests… or we could keep it simple like this until the need presents itself.

[jjb]

ah frack now 'helper' isn't in the load path

let me know if you like the file structure and if so i'll fix the require paths

[mislav]

ah frack now 'helper' isn't in the load path

The spec directory should be on the load path automatically. You shouldn't need to fix require paths. I'll look into what's going on here

[mislav]

Ah I see you've moved spec/helper.rb to spec/unit/. You shouldn't do that. Please make spec/helper.rb a common file that both styles of test suite can share.

[jjb]

@mislav fixed. see the new failures. how do we make access to those values compatible with old versions of faraday?

[mislav]

response.env[:url]

[jjb]

@mislav fixed, and tidied up some other related stuff

[jjb]

now it fails because coverage is too low… even though the only thing this PR does is add new tests, hah!

[mislav]

Yeah, bizarre. Please see if there are low-hanging fruit with regards to code coverage. We could even lower the treshold but that's only a temporary solution, as future PRs will have to lower the treshold even further, and that gets us nowhere.

[jjb]

@mislav see #100

[jjb]

I think because this PR only has new tests, a good case can be made that the coverage tool isn't correctly assessing the situation and that we should lower the threshold.

[mislav]

Yep, do it.

[jjb]

i've no idea what the cause of these new failures is...

[mislav]

It picked up simple_oauth 0.3.0 when previously we used simple_oauth 0.2.0. We have to look into the incompatibilities with the new version, but for now you could just put a version constraint for < 0.3.0 in the Gemfile.

[jjb]

😹

[mislav]

Thank you! Could you be bothered to look into the simple_oauth incompatibility thing or should we do it?

[jjb]

i've no experience with it and don't use it so it would be hard for me to prioritize, tbh