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#undefined_instance_methods #1026

Merged

Conversation

AI-Mozi
Copy link
Contributor

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

#1016
[Feature #12655]

Module#undefined_instance_methods has been added.

I hope all the most important test cases are included.

@AI-Mozi AI-Mozi force-pushed the add_specs_for_module_undef_inst_meth branch from 9dd7991 to a86f5aa Compare May 12, 2023 13:00
@AI-Mozi AI-Mozi marked this pull request as ready for review May 12, 2023 14:12
Copy link
Member

@andrykonchin andrykonchin left a comment

Choose a reason for hiding this comment

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

Looks good!

@andrykonchin
Copy link
Member

suggestion: Personally I prefer to avoid sharing fixture classes.

ParentClass, ChildClass etc are used in some other cases and have other reasons to change, not related to these specs. That may potentially make Module#undefined_instance_methods specs false positive, e.g. in

    it "does not returns ancestors undefined methods" do
      methods = ModuleSpecs::Grandchild.undefined_instance_methods
      methods.should_not include(:parent_method, :another_parent_method)
    end

the ModuleSpecs::Grandchild class might be changed and not have these methods undefined, or even not have these methods at all.

So I would create a separate hierarchy of classes (and possibly modules) what explicitly belong to the Module#undefined_instance_methods specs, for instance they could have "explicit" namespace - ModuleSpecs::UndefinedInstanceMethods and look like ModuleSpecs::UndefinedInstanceMethods::Parent, ModuleSpecs::UndefinedInstanceMethods::Child etc.

@AI-Mozi AI-Mozi force-pushed the add_specs_for_module_undef_inst_meth branch from a86f5aa to 6a6c490 Compare May 15, 2023 09:49
@andrykonchin
Copy link
Member

Thank you!

@andrykonchin andrykonchin merged commit 83b5dda into ruby:master May 15, 2023
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