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

Update lib/rspec/core/example_group.rb #790

Closed
wants to merge 1 commit into from
Closed

Update lib/rspec/core/example_group.rb #790

wants to merge 1 commit into from

Conversation

EarthCitizen
Copy link

"action" alias for describe:

The purpose of this is to declare a description of some kind of action which is performed repeatedly on many sub-items with out "abusing" context and without needing to alias it_behaves_like and calling that repeatedly.

One situation where this would be useful would be an auditing database trigger that inserts a record into an auditing table for each column of the record that the trigger was fired on.

Currently it would be done something like this:

describe "My Trigger" do
    context "When an update is done on a record" do
        it_performs "an audit", "COLUMN_ONE"
        it_performs "an audit", "COLUMN_TWO"
        it_performs "an audit", "COLUMN_THREE"
    end
end

Which outputs along the lines of:

My Trigger
    When an update is done on a record
        performs an audit
            On COLUMN_ONE
        performs an audit
            On COLUMN_TWO
        performs an audit
            On COLUMN_THREE

What the action alias would do is allow you to declare that a required action should take place, but to not repeat this for every item the action should happen for. Which would allow for the following:

describe "My Trigger" do
    context "When an update is done on a record" do
        action "performs an audit" do
            include_examples "test the audit results", "COLUMN_ONE"
            include_examples "test the audit results", "COLUMN_TWO"
            include_examples "test the audit results", "COLUMN_THREE"
        end
    end
end

Which would output along the lines of:

My Trigger
    When an update is done on a record
        performs an audit
            On COLUMN_ONE
            On COLUMN_TWO
            On COLUMN_THREE

While there my be other benefits to this, the main purpose behind this idea is to enhance readability.

"action" alias for describe:

The purpose of this is to declare a description of some kind of action which is performed repeatedly on many sub-items with out "abusing" context and without needing to alias it_behaves_like and calling that repeatedly.

One situation where this would be useful would be an auditing database trigger that inserts a record into an auditing table for each column of the record that the trigger was fired on.

Currently it would be done something like this:

describe "My Trigger" do
    context "When an update is done on a record" do
        it_performs "an audit", "COLUMN_ONE"
        it_performs "an audit", "COLUMN_TWO"
        it_performs "an audit", "COLUMN_THREE"
    end
end

Which outputs along the lines of:

My Trigger
    When an update is done on a record
        performs an audit
            On COLUMN_ONE
        performs an audit
            On COLUMN_TWO
        performs an audit
            On COLUMN_THREE

What the action alias would do is allow you to declare that a required action should take place, but to not repeat this for every item the action should happen for. Which would allow for the following:

describe "My Trigger" do
    context "When an update is done on a record" do
        action "performs an audit" do
            include_examples "test the audit results", "COLUMN_ONE"
            include_examples "test the audit results", "COLUMN_TWO"
            include_examples "test the audit results", "COLUMN_THREE"
        end
    end
end

Which would output along the lines of:

My Trigger
    When an update is done on a record
        performs an audit
            On COLUMN_ONE
            On COLUMN_TWO
            On COLUMN_THREE

While there my be other benefits to this, the main purpose behind this idea is to improve readability.
@myronmarston
Copy link
Member

I can see the readability benefits in your specific case, but I'm dubious of the general applicability of action. I can't think of a single case in my multi-year usage of RSpec where I would have benefitted from this.

I was also just thinking about the fact that we have alias_example_to:

https://www.relishapp.com/rspec/rspec-core/docs/configuration/alias-example-to

What do you think about adding an alias_describe_to option? It would allow you to configure action as an alias for your project. Also, it would add a nice bit of extra power in that, like alias_example_to, it would allow you to attach additional metadata to the example group automatically.

Thoughts?

@EarthCitizen
Copy link
Author

I that that alias_describe_to is a PERFECT solution. Better than adding a new alias to core. That possibility did not occur to me. Yes, lets proceed with that.

@dchelimsky
Copy link
Contributor

I'd recommend alias_example_group_to to align better with alias_example_to.

@dchelimsky
Copy link
Contributor

I think it's a great idea, btw.

@myronmarston
Copy link
Member

I agree--that's more consistent and reads better.

Sent from my iPhone

On Feb 7, 2013, at 9:05 AM, David Chelimsky notifications@github.com wrote:

I'd recommend alias_example_group_to to align better with alias_example_to.


Reply to this email directly or view it on GitHub.

@EarthCitizen
Copy link
Author

So alias_example_group_to would be something like this?

def self.alias_example_group_to(alias)
   alias_method alias, :describe
end

@dchelimsky
Copy link
Contributor

I'd actually recommend making example_group the primary method and then:

def self.alias_example_group_to(alias)
  alias_method alias, :example_group
end

alias_example_group_to :describe

Another issue to consider here: describe is a top level method, so you may want to scope this to nested groups. At one point we talked about adding RSpec.describe ... for those who prefer not to pollute the top level, so perhaps this is a good opportunity to introduce that as well, and then alias_example_group_to would make the alias available in nested group or top-level but only hanging off RSpec.

WDYT?

@EarthCitizen
Copy link
Author

Yeah, that way even describe itself is a kind of something, which in reality it is.

@myronmarston
Copy link
Member

One thing we'll want with alias_example_group_to is the ability to pass metadata that is automatically attached to example groups created using the aliased method. You can see how we do that for alias_example_to:

https://github.com/rspec/rspec-core/blob/master/lib/rspec/core/configuration.rb#L546-L570

Another issue to consider here: describe is a top level method, so you may want to scope this to nested groups.

I agree with scoping the methods created by this to only nested groups and not at the top level.

At one point we talked about adding RSpec.describe ... for those who prefer not to pollute the top level, so perhaps this is a good opportunity to introduce that as well, and then alias_example_group_to would make the alias available in nested group or top-level but only hanging off RSpec.

FWIW, it seemed much more important to provide a way to not pollute the top level back before my change in de01e05 -- that kept describe available at the top level, but prevented it from being added to every object, which is the real problem with most things that define top-level methods. I'm not sure if adding RSpec.describe (and the aliases created by this feature) is enough of a win to warrant adding the code for that--particularly since you would only really achieve the benefits of not polluting the main object by introducing yet another config option to disable adding describe to the main object.

@dchelimsky
Copy link
Contributor

I'd be very wary of producing top-level methods with alias_example_group_to even if they don't get added to every object. They'd still potentially conflict w/ top-level methods in other libs and apps.

@dchelimsky
Copy link
Contributor

I also think we can preserve describe on the main object. Default alias_example_group_to scope it to RSpec and nested groups, and then alias_example_group_to :describe, :expose_to_main_object => true or some such. Gives end users the option to expose aliases to main object, but they have to be explicit to do so.

@myronmarston
Copy link
Member

I'd be very wary of producing top-level methods with alias_example_group_to even if they don't get added to every object. They'd still potentially conflict w/ top-level methods in other libs and apps.

Right...I was trying to agree with this when I said:

I agree with scoping the methods created by this to only nested groups and not at the top level.

Default alias_example_group_to scope it to RSpec and nested groups, and then alias_example_group_to :describe, :expose_to_main_object => true or some such. Gives end users the option to expose aliases to main object, but they have to be explicit to do so.

I dislike alias_example_group_to :describe, :expose_to_main_object => true because normally the 2nd hash arg will define metadata that automatically applies to groups created with the named alias....but here, we'd have to special-case the hash key :expose_to_main_object to mean and do something different.

Gives end users the option to expose aliases to main object, but they have to be explicit to do so.

Currently we have only describe available at the top level, even though we have the context alias, and, to the best of my knowledge, no users have ever requested making context available at the top level, too. Maybe I'm just not being imaginative enough, but I'm having a hard time thinking of a situation where you'd want or need another alias at the top level. In a case like this action alias, it really only seems to make sense for a nested group. I'd prefer not to add the extra complexity of conditionally making the aliases available at the top level. Without that feature, the code for this is much more trivial, I think.

@dchelimsky
Copy link
Contributor

I'm having a hard time thinking of a situation where you'd want or need another alias at the top level.

Very few people will choose to use alias_example_group_to, but those who do will do so because they are experimenting with different language. To make this easy everywhere but the top level doesn't make sense to me, even if you or I can't come up with a specific example.

@EarthCitizen
Copy link
Author

Who knows what situation someone might think up. And could happen that someone wants implement entirely different aliases based on some type of DSL.

@myronmarston
Copy link
Member

Who knows what situation someone might think up. And could happen that someone wants implement entirely different aliases based on some type of DSL.

True. I'm still concerned about treating one of the entries of the metadata hash differently, but I'm not sure what (if anything) can be done about that. @EarthCitizen -- do you plan to submit a PR for this? (I'd like to close this one since the solution we've discussed is entirely different).

@EarthCitizen
Copy link
Author

Closing the issue. Much better solution presented.

@myronmarston
Copy link
Member

There's actually a PR up for this now: #870.

@EarthCitizen
Copy link
Author

Sounds good.

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