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

Cop suggestion: disallow return in tests #1237

Closed
ghiculescu opened this issue Feb 13, 2024 · 2 comments
Closed

Cop suggestion: disallow return in tests #1237

ghiculescu opened this issue Feb 13, 2024 · 2 comments

Comments

@ghiculescu
Copy link
Contributor

ghiculescu commented Feb 13, 2024

Is your feature request related to a problem? Please describe.

Consider this contrived example:

  test "foo" do
    users = User.all.select do |user|
      return false unless user.first_name == "Alex"
      user.last_name == "G"
    end
    assert_not_empty users
  end

If you glance at it you'd expect the test to pass or fail. But in reality, it returns after the second line. If you're catching assertionless tests you might catch it, but if not then you'll think it passes even when the assert line never gets hit.

Describe the solution you'd like

A linter that disallows using return in tests.

Note that this should only apply in tests that use test and a block. So while this test is also buggy, it should be accepted, since there is a valid use case for return in a method-based test (I think... I don't use this syntax).

  def test_foo
    users = User.all.select do |user|
      return false unless user.first_name == "Alex"
      user.last_name == "G"
    end
    assert_not_empty users
  end

Describe alternatives you've considered

I'm not sure on any other ways to catch this but would appreciate suggestions.

@pirj
Copy link
Member

pirj commented Feb 16, 2024

It seems that the problem is in the unintuitive behaviour of the return statement inside a block.
I can still see usefulness of guard statements in tests.

@koic
Copy link
Member

koic commented May 6, 2024

It's hard to say whether this cop is useful for everyone, as it is not designed just for Rails. Therefore, I'll close this issue. Thanks for the suggestion.

@koic koic closed this as not planned Won't fix, can't repro, duplicate, stale May 6, 2024
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

No branches or pull requests

3 participants