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

New cop: Minitest/PrivateTestMethod #220

Closed
fatkodima opened this issue Jan 5, 2023 · 4 comments · Fixed by #229
Closed

New cop: Minitest/PrivateTestMethod #220

fatkodima opened this issue Jan 5, 2023 · 4 comments · Fixed by #229
Labels
enhancement New feature or request

Comments

@fatkodima
Copy link
Contributor

While adding a new test case to a large codebase (rails/rails as an example) with its test files sometimes having thousands of lines of code and not following TDD (as I believe most of people do, mine included, except for bugfixes), it is very easy to add a test case to the private class section. The test case won't be run and the developer will think that tests are passing.

# bad
class FooTest < Minitest::Test
  def test_foo; end

  private
  def test_bar
  end
end

# good
class FooTest < Minitest::Test
  def test_foo; end
  def test_bar; end
end

# good
class FooTest < Minitest::Test
  def test_foo; end

  private
  def bar
  end
end
@koic
Copy link
Member

koic commented Jan 6, 2023

I agree that this is probably out of the xUnit execution pattern. However, I'm not sure if that was intentional or a mistake by user. First of all, it may be better to specify it in Minitest Style Guide as it will become a stronger basis as a rule:
https://github.com/rubocop/minitest-style-guide

@andyw8
Copy link
Contributor

andyw8 commented Jan 6, 2023

I'd say that if the test name begins test_ then it's reasonable to assume it was a mistake to make it private.

@fatkodima
Copy link
Contributor Author

I think, we could also check for assertions inside that method. In this case it is 100% a mistake.

@koic
Copy link
Member

koic commented Jan 6, 2023

Okay, It doesn't have to be checked inside the method. Note, protected should also be treated the same as private. So, the cop name can be adjusted if we get good inspiration.

@koic koic added the enhancement New feature or request label Jan 6, 2023
@koic koic closed this as completed in #229 Jan 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants