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 test for view "Helper" module to improve coverage #738

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kenboo0426
Copy link

Hi, Thank you for the great Gem!
I often use this gem!

Added Helper.policy_scope test to improve maintainability.
As a result, the test coverage changed from 98.41% to 100% in lib/pundit.rb

If there is a place to fix, please let me know.
best regards.

@kenboo0426 kenboo0426 changed the title Add test module "Helper" Add test for module "Helper" May 21, 2022
Comment on lines +220 to +225
include Pundit::Authorization
include Pundit::Helper
# Mark protected methods public so they may be called in test
# rubocop:disable Style/AccessModifierDeclarations
public(*Pundit::Authorization.private_instance_methods)
# rubocop:enable Style/AccessModifierDeclarations
Copy link
Member

Choose a reason for hiding this comment

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

Hi! Sorry about coming in this late.

I don't think we need to make any private instance methods public at all? Also the comment refers to protected methods but this does private methods.

What about this?

Suggested change
include Pundit::Authorization
include Pundit::Helper
# Mark protected methods public so they may be called in test
# rubocop:disable Style/AccessModifierDeclarations
public(*Pundit::Authorization.private_instance_methods)
# rubocop:enable Style/AccessModifierDeclarations
def self.helper(mod)
include(mod)
end
include Pundit::Authorization

@Burgestrand Burgestrand changed the title Add test for module "Helper" Add test for view "Helper" module to improve coverage May 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants