troessner / reek

Code smell detector for Ruby
https://github.com/troessner/reek
MIT License
4.04k stars 280 forks source link

Cucumber problems #1416

Closed troessner closed 6 years ago

troessner commented 6 years ago

A couple of days suddenly lot of cucumber scenarios failed for me the like this:

expected that command "reek --no-color --no-documentation --documentation smelly.rb" has exit status of "2", but has "0". (RSpec::Expectations::ExpectationNotMetError)
./features/step_definitions/reek_steps.rb:37:in `/^the exit status indicates smells$/'
features/reports/reports.feature:192:in `Then the exit status indicates smells'
features/reports/reports.feature:181:in `Then the exit status indicates smells'

expected that command "reek --no-color --no-documentation --no-documentation smelly.rb" has exit status of "2", but has "0". (RSpec::Expectations::ExpectationNotMetError)
./features/step_definitions/reek_steps.rb:37:in `/^the exit status indicates smells$/'
features/reports/reports.feature:197:in `Then the exit status indicates smells'

expected that command "reek --no-color --no-documentation --no-line-numbers -U smelly.rb" has exit status of "2", but has "0". (RSpec::Expectations::ExpectationNotMetError)
./features/step_definitions/reek_steps.rb:37:in `/^the exit status indicates smells$/'
features/reports/reports.feature:218:in `Then the exit status indicates smells'
features/reports/reports.feature:208:in `Then the exit status indicates smells'

This happens on my local master for me and it's very weird. I did not change anything in my local set up. Travis is still green (also for fresh pull requests) so apparently it is a local problem. The thing is, i have not the slightest clue what it is.

What I tried:

I can't recall any OS-related updates over the last couple of days.

I do have the suspicion that it might have to do sth with readline and rb-readline but I'm not sure how to debug this further.

I'd appreciate any ideas at this point ;)

@mvz @chastell I assume this doesn't happen for you, does it? If not, could you tell what OS you are using? I'm using High Sierra 10.13.6.

mvz commented 6 years ago

What I tend to do in such cases is just run the one failing scenario, and then cd into tmp/aruba and run, e.g., bundle exec reek --no-color --no-documentation --documentation smelly.rb there and see what broke.

mvz commented 6 years ago

The scenarios run fine here by the way. I'm running Linux (Debian Sid).

troessner commented 6 years ago

What I tend to do in such cases is just run the one failing scenario, and then cd into tmp/aruba and run, e.g., bundle exec reek --no-color --no-documentation --documentation smelly.rb there and see what broke.

Unfortunately this doesn't give me further hints:

( $ ) rm -r tmp/aruba/ && bx cucumber features/command_line_interface/basic_usage.feature:6

Feature: The Reek CLI maintains backwards compatibility
  In order to use Reek without fuss
  As a developer
  I want to have a stable basic command line interface

  Scenario: the example from README reports as expected # features/command_line_interface/basic_usage.feature:6
    Given the smelly file 'smelly.rb'                   # features/step_definitions/sample_file_steps.rb:3
    When I run reek smelly.rb                           # features/step_definitions/reek_steps.rb:1
    Then the exit status indicates smells               # features/step_definitions/reek_steps.rb:35
      expected that command "reek --no-color --no-documentation smelly.rb" has exit status of "2", but has "0". (RSpec::Expectations::ExpectationNotMetError)
      ./features/step_definitions/reek_steps.rb:37:in `/^the exit status indicates smells$/'
      features/command_line_interface/basic_usage.feature:9:in `Then the exit status indicates smells'
    And it reports:                                     # features/step_definitions/reek_steps.rb:44
      """
      smelly.rb -- 2 warnings:
        [4]:UncommunicativeMethodName: Smelly#x has the name 'x'
        [5]:UncommunicativeVariableName: Smelly#x has the variable name 'y'
      """

Failing Scenarios:
cucumber features/command_line_interface/basic_usage.feature:6 # Scenario: the example from README reports as expected

1 scenario (1 failed)
4 steps (1 failed, 1 skipped, 2 passed)
0m0.774s

And now let's run Reek:

( $ ) cd tmp/aruba && bundle exec reek --no-color --no-documentation smelly.rb

Inspecting 1 file(s):
.

Any other ideas? I'm out :-/

mvz commented 6 years ago

So no smells found but you expect smells, right? Which .reek.yml is used?

mvz commented 6 years ago

Perhaps there's a .reek.yml in tmp/?

troessner commented 6 years ago

So no smells found but you expect smells, right? Which .reek.yml is used?

Exactly, I expect smells to be found but there are none. "smelly.rb" looks correct and there is no .reek.yml to be found:

  ( $ (master)) pwd
/Users/timo/code/reek/tmp/aruba

  ( $ (master)) cat smelly.rb
# Smelly class
class Smelly
  # This will reek of UncommunicativeMethodName
  def x
    y = 10 # This will reek of UncommunicativeVariableName
  end
end

  ( $ (master)) ls -al
total 8
drwxr-xr-x  3 timo  staff   96 Sep 11 14:53 .
drwxr-xr-x  8 timo  staff  256 Sep 11 14:53 ..
-rw-r--r--  1 timo  staff  153 Sep 11 14:53 smelly.rb
troessner commented 6 years ago

This is super confusing. So bx cucumber features/command_line_interface/basic_usage.feature:6 expects smells but there arent any. This

( $ (master)) ../../bin/reek smelly.rb

Inspecting 1 file(s):
.

should report smells as well but it doesn't. Which hints to .reek.yml somewhere. Even more confusing, we have a .reek.yml in our project root folder. Which is empty. Seems this was introduced here and I kind of missed that in the review - why do we need an empty .reek.yml in our project root?

In any case, when I remove the file it doesn't change anything, cucumber still fails and bin/reek still says no smells to be found. ( I also have no .reek.yml in my ~).

Maybe the key to this conundrum is implementing #1382 first?

mvz commented 6 years ago

why do we need an empty .reek.yml in our project root?

To avoid the specs and cukes picking up some .reek.yml further up the directory tree.

Maybe the key to this conundrum is implementing #1382 first?

Yeah, that might help, but hopefully you can just binding.pry somewhere for now (when running reek straight from tmp/aruba).

troessner commented 6 years ago

Oh my, that was pretty stupid.

 ( $ ) find . -name '.reek.yml'
./code/reek/samples/configuration/regular_configuration/.reek.yml
./code/reek/tmp/.reek.yml

So no, there was no .reek.yml in tmp/aruba and there was no .reek.yml in the project directory but there was a .reek.yml .... in between in tmp/. I'll implement #1382 right away to prevent this from ever happening again.

To avoid the specs and cukes picking up some .reek.yml further up the directory tree.

The specs are not affected by this I believe. Just ran them without the .reek.yml and they succeeded.

mvz commented 6 years ago

Just ran them without the .reek.yml and they succeeded.

Yes, but you don't have a conflicting .reek.yml in home. We've had problems with contributors having failing specs because they had a non-default reek config in home.

troessner commented 6 years ago

Yes, but you don't have a conflicting .reek.yml in home. We've had problems with contributors having failing specs because they had a non-default reek config in home.

Fair point ;)