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

Add specs for Module#refinements and Refinement#refined_class #1027

Merged

Conversation

AI-Mozi
Copy link
Contributor

@AI-Mozi AI-Mozi commented May 15, 2023

#1016
[Feature #12737]

Module#refinements has been added

@AI-Mozi AI-Mozi force-pushed the add_specs_for_refinements_and_refined_class branch from 417cc29 to 7f77f08 Compare May 15, 2023 12:40
@AI-Mozi AI-Mozi marked this pull request as ready for review May 15, 2023 12:56
core/module/refinements_spec.rb Outdated Show resolved Hide resolved
core/module/refinements_spec.rb Outdated Show resolved Hide resolved
end
end

m.refinements.should == [refined_integer, refined_string]
Copy link
Member

Choose a reason for hiding this comment

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

minor: Not sure whether the order is guarantied. I suppose it isn't, so we may just sort both arrays, I mean:

Suggested change
m.refinements.should == [refined_integer, refined_string]
m.refinements.sort.should == [refined_integer, refined_string].sort

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure if m.refinemets.sort works

Copy link
Member

Choose a reason for hiding this comment

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

Hm, right. Will sorting by object_id or to_s be working?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, both are correct. Should I change it to
m2.refinements.sort_by(&:object_id).should == ScratchPad.recorded.sort_by(&:object_id) then?

Copy link
Member

Choose a reason for hiding this comment

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

Yes.

And in used_refinements (#1028) also order is not guarantied, so sorting probably also makes sense.

core/module/refinements_spec.rb Show resolved Hide resolved
core/refinement/refined_class_spec.rb Show resolved Hide resolved
@AI-Mozi AI-Mozi force-pushed the add_specs_for_refinements_and_refined_class branch from 7f77f08 to d8f052d Compare May 16, 2023 07:15
@AI-Mozi AI-Mozi force-pushed the add_specs_for_refinements_and_refined_class branch 3 times, most recently from 53b37be to 256f3ea Compare May 17, 2023 08:42
core/module/refinements_spec.rb Outdated Show resolved Hide resolved
core/module/refinements_spec.rb Outdated Show resolved Hide resolved
@AI-Mozi AI-Mozi force-pushed the add_specs_for_refinements_and_refined_class branch from 256f3ea to 354edd9 Compare May 17, 2023 10:27
@andrykonchin
Copy link
Member

Thank you for the specs!

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

Successfully merging this pull request may close these issues.

2 participants