-
Notifications
You must be signed in to change notification settings - Fork 12
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
Jacobperia component unit tests #85
base: main
Are you sure you want to change the base?
Conversation
jacobperia
commented
Oct 12, 2023
•
edited
Loading
edited
- Rspec tests for NewUsersComponent and NewActivityComponent
- Initialize in ApplicationComponent will validate arguments and throw errors for the required components.
2c6c793
to
0dfe187
Compare
@@ -2,8 +2,7 @@ | |||
|
|||
class Matey::ActiveUsersComponent < Matey::ApplicationComponent | |||
def initialize(events:, time_window: 1.week, color_scheme: "neutral") | |||
raise ArgumentError unless events.is_a?(ActiveRecord::Relation) | |||
raise ArgumentError unless time_window.is_a?(Integer) | |||
validate_arguments(events, time_window) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice way to try things up. I've not really thought about this type of validation before but seems to make sense and definitely better than including it in each file.
Thoughts on leveraging super
as an alternative implementation? Not sure it is better ...
In this implementation, I think it would be nice to have named arguments so that the method signature would be:
validate_arguments(events: events, time_window: time_window)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was able to implement both the suggestions. Much cleaner now 🚀