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

Alias example group #1255

Merged
merged 22 commits into from Jan 28, 2014

Conversation

Projects
None yet
3 participants
@myronmarston
Member

myronmarston commented Jan 22, 2014

This is a follow up to #1236. It's @michihuber's work plus the start of fixes I've applied based on mine and @JonRowe's feedback on that PR. This supercedes that PR.

@myronmarston myronmarston referenced this pull request Jan 22, 2014

Closed

Alias example group #1236

michihuber and others added some commits Apr 8, 2013

rename #describe to #example_group
Conflicts:
	lib/rspec/core/example_group.rb
Standardize on `__send__` over `send`.
We've occasionally received bug reports related to
users having objects that redefine `send` (e.g.
`Email#send`), and using `__send__` is best to avoid
those problems.

Many of the receivers of `__send__` here are objects
we own and that do not (and never will) redefine `send`;
however, for consistency, it's good to standardize on
`__send__`. That way, we're less likely to forget it in
the future for a case where it does matter.
Add missing spec.
This already worked but wasn't covered by a test.
Improve DSL specs.
- Cover all the edge cases.
- Use a subprocess for global config changes.
- No need to use a separate example group per method.
  It adds to the runtime of the test suite with no benefit.
Ensure example group aliases are exposed globally if so configured.
Before, they were not exposed globally if the config option
was already set.
Cleanup how the DSL is defined.
- No need to use send.
- Extract change_global_dsl helper method.
@michihuber

This comment has been minimized.

Show comment
Hide comment
@michihuber

michihuber Jan 23, 2014

Contributor

Thanks @myronmarston for taking over!
(I just came back online, which is why I couldn't continue working on this earlier.)

Contributor

michihuber commented Jan 23, 2014

Thanks @myronmarston for taking over!
(I just came back online, which is why I couldn't continue working on this earlier.)

@myronmarston

This comment has been minimized.

Show comment
Hide comment
@myronmarston

myronmarston Jan 24, 2014

Member

OK, I think this is ready to merge. @michihuber / @JonRowe -- care to review what I've done on top of #1236?

Member

myronmarston commented Jan 24, 2014

OK, I think this is ready to merge. @michihuber / @JonRowe -- care to review what I've done on top of #1236?

@myronmarston

This comment has been minimized.

Show comment
Hide comment
@myronmarston
Member

myronmarston commented Jan 28, 2014

Anyone want to review this?

/cc @JonRowe @xaviershay @samphippen @alindeman @soulcutter

RSpec.detail "a detail" do
it "can do some less important stuff" do
end
end

This comment has been minimized.

@JonRowe

JonRowe Jan 28, 2014

Member

This doesn't "prove" the :detailed => true part of the alias, do we care? Perhaps we could assert on the metadata?

Scratch that I see you're using tags.

@JonRowe

JonRowe Jan 28, 2014

Member

This doesn't "prove" the :detailed => true part of the alias, do we care? Perhaps we could assert on the metadata?

Scratch that I see you're using tags.

This comment has been minimized.

@myronmarston

myronmarston Jan 28, 2014

Member

Honestly, I'm not super happy with this cucumber example. Got a better idea?

@myronmarston

myronmarston Jan 28, 2014

Member

Honestly, I'm not super happy with this cucumber example. Got a better idea?

expect(main).to respond_to(*method_names)
expect(Module.new).to respond_to(*method_names)
expect(Object.new).not_to respond_to(*method_names)

This comment has been minimized.

@JonRowe

JonRowe Jan 28, 2014

Member

Did not know you could splat method names into that...

@JonRowe

JonRowe Jan 28, 2014

Member

Did not know you could splat method names into that...

Show outdated Hide outdated script/ignores Outdated
@JonRowe

This comment has been minimized.

Show comment
Hide comment
@JonRowe

JonRowe Jan 28, 2014

Member

Apart from my white space niggle, LGTM :P

Member

JonRowe commented Jan 28, 2014

Apart from my white space niggle, LGTM :P

myronmarston added a commit that referenced this pull request Jan 28, 2014

@myronmarston myronmarston merged commit 729f2f7 into master Jan 28, 2014

1 check passed

default The Travis CI build passed
Details

@myronmarston myronmarston deleted the pr-1236-rebased branch Jan 28, 2014

@xaviershay xaviershay changed the title from Pr 1236 rebased to Alias example group Apr 27, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment