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

Enable refremote specs #164

Merged
merged 3 commits into from
Oct 31, 2014
Merged

Enable refremote specs #164

merged 3 commits into from
Oct 31, 2014

Conversation

pd
Copy link
Contributor

@pd pd commented Oct 29, 2014

Enables the refRemote tests from the common test suite by using webmock to stub out requests to http://localhost:1234/<foo>. After this, there's only one mandatory test remaining (draft3 disallow) for compliance with the common suite (see #163).

Two annoyances here, but I think both are minor enough to be acceptable for now:

  1. The webmock usage currently bleeds into another test (test_bad_schema_ref), where we need to explicitly re-enable network access in order for the expected error to be raised. It's just the one file (so far?), so I didn't spend much time trying to address this. webmock seems to stub out Net::HTTP as soon as it is required.
  2. A test run is somewhat slower now: from ~1.5s to ~2.5s on my machine. Probably an acceptable tradeoff, but a bit disappointing. =)

@hoxworth
Copy link
Contributor

LGTM

@iainbeeston
Copy link
Contributor

Argh... It'd be good to get the test run time down. Doesn't have to be in this pull request though=

@pd
Copy link
Contributor Author

pd commented Oct 29, 2014

@iainbeeston see my comment in the "optional autoloading" issue; I think that feature should bring the test time back down, since it would allow us to revert webmock usage. In the meantime, this gets us closer to #163 at least. =)

@RST-J
Copy link
Contributor

RST-J commented Oct 29, 2014

👍

@pd pd force-pushed the enable-refremote-specs branch from 3977668 to 879a8a1 Compare October 30, 2014 13:03
@pd
Copy link
Contributor Author

pd commented Oct 30, 2014

Rebased to fix merge conflicts now that #171 landed.

@pd pd mentioned this pull request Oct 30, 2014
4 tasks
RST-J added a commit that referenced this pull request Oct 31, 2014
@RST-J RST-J merged commit 6f16522 into voxpupuli:master Oct 31, 2014
@pd pd deleted the enable-refremote-specs branch November 1, 2014 14:38
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.

4 participants