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

Jump to bottom

Add docs to expectations methods #5092

Merged
merged 4 commits into from Oct 26, 2017
Merged

Add docs to expectations methods #5092

merged 4 commits into from Oct 26, 2017

Conversation

straight-shoota
Copy link
Member

No description provided.

@straight-shoota straight-shoota mentioned this pull request Oct 9, 2017
Closed
RX14
RX14 requested changes Oct 9, 2017
View reviewed changes
Copy link
Contributor

@RX14 RX14 left a comment

Choose a reason for hiding this comment

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

In general there doesn't seem to be much documentation on what an Expectation is and how to use it (should)

src/spec/expectations.cr Outdated
def be_close(expected, delta)
Spec::CloseExpectation.new(expected, delta)
end

# Returns a factory to create a comparison `Expectation` that:
#
# * passes if actual is lesser than *value*: `be.<(value)`
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps using the longer be < value is clearer here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. For some reason I assumed the . was require (dunno why, something failed when I was experimenting with it).

src/spec/expectations.cr Outdated
macro be_a(type)
Spec::BeAExpectation({{type}}).new
end

# Creates an `Expectation` that passes if the block raises an exception.
Copy link
Contributor

Choose a reason for hiding this comment

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

These expect_* macros don't create an expectation, they raise themselves.

yxhuvud
yxhuvud reviewed Oct 10, 2017
View reviewed changes
src/spec/expectations.cr Outdated
macro be_a(type)
Spec::BeAExpectation({{type}}).new
end

# Runs the block and passes if it raises an exception.
Copy link
Contributor

@yxhuvud yxhuvud Oct 10, 2017 •
edited

Choose a reason for hiding this comment

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

Worth noting about this method is that this variant that doesn't verify it against an actual exception type has been deprecated in rspec. From the places where I fixed the deprecation errors for that, I can say that the reason for the deprecation was very motivated, as there was lots of hidden errors and unexpected exceptions being accepted when they really shouldn't have.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point! I'll add a note about this and suggest removing the matcher in a separate issue.

@straight-shoota
Copy link
Member Author

@RX14 Yes, there is still a lot missing. I'll try to condense some of the testing guide to a API doc for Spec.

@straight-shoota straight-shoota mentioned this pull request Oct 10, 2017
Merged
@RX14
Copy link
Contributor

RX14 commented Oct 10, 2017 •
edited

@straight-shoota just "Expectations are used by Spec::ObjectExtensions#should and Spec::ObjectExtensions#should_not." would be enough clarification. Some doc on those two methods and that module would be great too.

@straight-shoota
Copy link
Member Author

@RX14 The module Spec itself is already documented, that's what I used as a starting point for the testing guide.
I'll try to add documentation to the methods.

@straight-shoota straight-shoota force-pushed the jm-expectations-doc branch 2 times, most recently from 3fb6bb0 to a33223f Compare October 11, 2017 13:21
@RX14
Copy link
Contributor

RX14 commented Oct 14, 2017

Needs a rebase.

@straight-shoota
Copy link
Member Author

Ready to merge! 💯

RX14
RX14 requested changes Oct 15, 2017
View reviewed changes
src/spec.cr Outdated
@@ -1,13 +1,16 @@
require "./spec/dsl"

# Crystal's built-in testing library.
# Crystal's built-in testing library. It provides the structure for writing executable examples
Copy link
Contributor

Choose a reason for hiding this comment

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

"a structure"

src/spec.cr Outdated
@@ -1,13 +1,16 @@
require "./spec/dsl"

# Crystal's built-in testing library.
# Crystal's built-in testing library. It provides the structure for writing executable examples
# of how your code should behave. A domain specific language allows to write them similar to natural language.
Copy link
Contributor

Choose a reason for hiding this comment

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

"allows you to write them in a way similar to natural language"

@straight-shoota straight-shoota mentioned this pull request Oct 18, 2017
Merged
@Sija
Copy link
Contributor

Sija commented Oct 26, 2017

Ready to 🎲 !

@RX14 RX14 merged commit 844cc6e into crystal-lang:master Oct 26, 2017
@RX14 RX14 added this to the Next milestone Oct 26, 2017
@bew bew mentioned this pull request Dec 22, 2017
Closed
@straight-shoota straight-shoota deleted the jm-expectations-doc branch December 22, 2017 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yxhuvud yxhuvud yxhuvud left review comments

@asterite asterite asterite approved these changes

@RX14 RX14 RX14 approved these changes

Assignees
No one assigned
Labels
kind:docs topic:stdlib
Projects
None yet
Milestone
0.24.0
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@straight-shoota @RX14 @Sija @yxhuvud @asterite