Skip to content

Conversation

@roback
Copy link
Member

@roback roback commented Jan 27, 2016

Made some small improvements to the tests.

in the spec for #endpoint_url.

It also didn't fit inside the describe block for the .new method.
The same string was used everywhere.
* Changed some context descriptions
* Use expect_any_instance instead of allow_any_instance
* Put user_agent spec after the block spec so all API key specs
  are grouped together.
They were only used in two places in the file.
No need to repeat "Fixture.get" for each test.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

noticed we changed to described_class in another test, do it here too?

(although I think we should use some other more appropriate matcher)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just thought it looks weird checking that described_class.new is a described_class. It's easier to understand if its explicit, but the "correct way" would probably be to use described_class here too.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright

@walro
Copy link
Contributor

walro commented Jan 27, 2016

I think this was an improvement, merge?

@roback
Copy link
Member Author

roback commented Jan 27, 2016

Yes, I'll merge.

roback added a commit that referenced this pull request Jan 27, 2016
@roback roback merged commit bac8e4a into master Jan 27, 2016
@roback roback deleted the improve-tests branch January 27, 2016 15:35
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.

3 participants