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 intentionally empty file to implicitly test cops on it #11473

Merged
merged 1 commit into from
Jan 20, 2023

Conversation

fatkodima
Copy link
Contributor

Here and there are error reports that some cop fails on an empty file. The latest of them - rubocop/rubocop-minitest#225. Some of the cops already have test cases for empty files, but it is tedious to write them and very often just forgotten.

So I added an empty file into the project so that cops (at least which are not disabled) are always tested (implicitly) on an empty file.
I think, we should add a similar file to the test/ directory of the rubocop-minitest gem and somewhere to the rubocop-rails too.

@bbatsov
Copy link
Collaborator

bbatsov commented Jan 19, 2023

I get the idea, but I don't like having an empty lib file simply for test purposes (as it will look weird if someone's browsing the code). I guess we can just have the test task create and delete one or something along those lines.

@fatkodima
Copy link
Contributor Author

That is actually the response I was expected 😄

How about moving it into the spec/ directory? Or tweaking spec/spec_helper.rb to add such a test case to all cop test suites? (not sure how to do it - not a rspec user).

@bbatsov
Copy link
Collaborator

bbatsov commented Jan 19, 2023

Something like this would be fine by me.

@fatkodima
Copy link
Contributor Author

Moved the file to spec/.

@bbatsov bbatsov merged commit 556b331 into rubocop:master Jan 20, 2023
@fatkodima fatkodima deleted the empty-file branch January 20, 2023 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants