Use a more minimal require pattern for linter specs#9756
Merged
Conversation
The rubocop linter specs were requiring the `rubocop/rspec/support`. This file is [defined in the Rubocop gem](https://github.com/rubocop/rubocop/blob/master/lib/rubocop/rspec/support.rb) and adds utilities for testing rubocop linters. The rubocop support configuration _appears_ to make the assumption that the context in which it is included is a test suite for only testing Cops or at least for primarily testing Cops. The main clue for this is this part of the implementation: ```ruby RSpec.configure do |config| config.include CopHelper # ... end ``` The [`CopHelper` module](https://github.com/rubocop/rubocop/blob/master/lib/rubocop/rspec/cop_helper.rb) defines a number of methods for testing a linter. The support file includes that module for _every single example_. As a result all of our tests have those methods. I found this problematic when I observed an issue testing a MFA selection presenter. It referenced a `configuration` variable that it expected to be an MFA configuration. This variable was in fact a rubocop configuration thanks to the `configuration` method in the `CopHelper` module (ref: [the failed build where I observed this](https://gitlab.login.gov/lg/identity-idp/-/jobs/915207)) This commit removes the `require` call that included `rubocop/rspec/support`. It replaces that with a simplified set of `require` calls that only bring in the tools we need to test our linters. [skip changelog]
jmhooper
changed the title
mitchellhenke
approved these changes
Dec 13, 2023
Contributor
|
🤔 Interesting discovery! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The rubocop linter specs were requiring the
rubocop/rspec/support. This file is defined in the Rubocop gem and adds utilities for testing rubocop linters.The rubocop support configuration appears to make the assumption that the context in which it is included is a test suite for only testing Cops or at least for primarily testing Cops. The main clue for this is this part of the implementation:
The
CopHelpermodule defines a number of methods for testing a linter. The support file includes that module for every single example. As a result all of our tests have those methods.I found this problematic when I observed an issue testing a MFA selection presenter. It referenced a
configurationvariable that it expected to be an MFA configuration. This variable was in fact a rubocop configuration thanks to theconfigurationmethod in theCopHelpermodule (ref: the failed build where I observed this)This commit removes the
requirecall that includedrubocop/rspec/support. It replaces that with a simplified set ofrequirecalls that only bring in the tools we need to test our linters.