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

Refactor transient registration unit tests #313

Merged
merged 2 commits into from
Nov 28, 2018

Conversation

Cruikshanks
Copy link
Member

Started by following http://www.betterspecs.org/#describe and ensuring where we are describing an instance method we start the description with a #.

However then to make this task a little easier, decided to reorder the tests slightly so that shared examples come first, then anything related to attributes, followed by the instance methods.

So did that, and the # in descriptions, but then spotted we were testing a method that actually came from a concern and was not directly on the TransientRegistration class.

This should really be in a shared example to make it clearer its not directly tied to the transient registration class, so also made that tweak.

Started by following http://www.betterspecs.org/#describe and ensuring where we are describing an instance method we start the description with a #.

However then to make this task a little easier, decided to reorder the tests slightly so that shared examples come first, then anything related to attributes, followed by the instance methods.

So did that, and the # in descriptions, but then spotted we were testing a method that actually came from a concern and was not directly on the `TransientRegistration` class.

This should really be in a shared example to make it clearer its not directly tied to the transient registration class, so also made that tweak.
@Cruikshanks Cruikshanks self-assigned this Nov 28, 2018
Copy link
Member

@irisfaraway irisfaraway left a comment

Choose a reason for hiding this comment

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

LGTM. I'll try to fix the # thing when I see it elsewhere as I know this isn't the only place it was inconsistent.

@Cruikshanks Cruikshanks merged commit 5ae6991 into master Nov 28, 2018
@Cruikshanks Cruikshanks deleted the refactor-transient-registration-tests branch November 28, 2018 16:32
@irisfaraway irisfaraway added the housekeeping Changes such as refactoring label Dec 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
housekeeping Changes such as refactoring
Projects
None yet
2 participants