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

Refactored request specs. #29

Merged
merged 1 commit into from Aug 10, 2014

Conversation

cameron-martin
Copy link
Contributor

I introduced a stub_request method to simplify the fixture loading and method expectations. I also moved the message expectations out of the subject block and into a before filter, since I believe expectations do not belong in a subject block.

This simplifies this

subject do
  expect(request.class).to receive(:get).with(request.api_url("game/by-summoner/1/recent")).and_return load_fixture('game', GameRequest.api_version, 'get')

  request.recent 1
end

to this

subject { request.recent(1) }

before(:each) { stub_request(request, 'game', 'game/by-summoner/1/recent') }

which I believe is more readable. It also has the added benefit of making changing the tests easier if you ever decide to use a different http library.

The http GET verb is hard-coded into stub_request, but the API is currently does not, and is unlikely to ever, use any other http verb.

I changed the name of the getbyid-team fixture to get-team-by-id, to be consistent with get-summoner-by-name and get-champion-by-id, etc. This also makes the above simplifications possible.

I also moved the spec helper methods previously defined in spec_helper into a module in support/helpers. The makes the spec_helper file less cluttered. The two files named helpers and model_helpers may be confusing; if you can think of any better names I'll change them.

There are a few other, mainly cosmetic/structural, changes that I made to the request specs.

If there is anything you don't agree with, I'd be happy to discuss/revert it :)

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) when pulling 92a7357 on cameron-martin:refactor-request-spec-2 into fe2d0e4 on mikamai:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) when pulling 4c39b02 on cameron-martin:refactor-request-spec-2 into fe2d0e4 on mikamai:master.

@cameron-martin
Copy link
Contributor Author

Hmm, not sure how the coverage decreased. This was supposed to be just a refactor.

@cameron-martin
Copy link
Contributor Author

Although it is longer, a more descriptive name for stub_request might be stub_and_expect_request, or even just expect_request. Let me know if you want that changed.

intinig added a commit that referenced this pull request Aug 10, 2014
@intinig intinig merged commit 84fab1b into mikamai:master Aug 10, 2014
@intinig
Copy link
Contributor

intinig commented Aug 10, 2014

I really like this refactoring, thank you ;)

I've been thinking for a while the specs needed some refactoring but never got around doing them.

Good job!

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.

None yet

3 participants