feat: add rspec rubocop #290
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR improves the style and consistency of the test suite while updating several instances of test doubles to use stricter instance doubles, and it adds additional RuboCop configuration for RSpec. It also includes minor functional improvements such as aliasing methods in event classes and updates to documentation and gemspec dependencies.
- Updated test doubles to use instance_double for stronger type checking.
- Renamed spec descriptions and commands for better clarity.
- Added new RuboCop configurations for RSpec and made minor refactoring in library files.
Reviewed Changes
Copilot reviewed 30 out of 30 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| spec/**/*.rb | Updated tests to use instance_double and refactored spec names and expectations for clarity |
| lib/discordrb/permissions.rb | Simplified bitwise check in defined_permissions |
| lib/discordrb/events/* | Introduced alias_method :matching? to simplify event matching logic |
| discordrb.gemspec & .rubocop.yml | Added rubocop-rspec dependency and updated RuboCop configuration for RSpec |
| ].freeze | ||
|
|
||
| # TODO: Rework tests | ||
| # rubocop:disable RSpec/MultipleExpectations |
There was a problem hiding this comment.
[nitpick] Consider refactoring these tests to reduce the number of multiple expectations per test case. This would improve test clarity and maintainability over time.
| # rubocop:disable RSpec/MultipleExpectations |
| expect { described_class.new(token: nil) }.to raise_error('Token string is empty or nil') | ||
| shared_examples 'raises when token string is' do |token, value| | ||
| it "raises when token string is #{token}" do | ||
| expect { described_class.new(token: value) }.to raise_error('Token string is empty or nil') |
There was a problem hiding this comment.
[nitpick] To improve maintainability, consider extracting the error message ('Token string is empty or nil') to a constant so that tests remain in sync if the error message changes in the future.
| expect { described_class.new(token: value) }.to raise_error('Token string is empty or nil') | |
| expect { described_class.new(token: value) }.to raise_error(TOKEN_ERROR_MESSAGE) |
| end | ||
|
|
||
| # TODO: Rework tests to not rely on multiple expectations | ||
| # rubocop:disable RSpec/MultipleExpectations |
There was a problem hiding this comment.
[nitpick] Review and refactor tests that require disabling the 'RSpec/MultipleExpectations' cop to reduce complexity and improve overall test quality.
1 participant
Summary
Aims to begin to improve upon and homogenise the style of rspec tests by adding rubocop rules
There are some TODO actions that require more thought to iterate on than in this PR
There is also room to discuss rules to configure. This PR is an entry into that discussion as it demonstrates the required changes needed to abide by the rules.
There are some rubocops that will fail in CI which are fixed in another PR
Added
Fixed