enkessler / cuke_linter

A linting tool for Cucumber
MIT License
32 stars 8 forks source link

Feature/lint warn steps too long #7

Closed BillyRuffian closed 5 years ago

BillyRuffian commented 5 years ago

Hello, step too long linter using 80 chars as the default max (as the old gherkin_lint used)

BillyRuffian commented 5 years ago

Hello, step too long linter using 80 chars as the default max (as the old gherkin_lint used)

enkessler commented 5 years ago

Hmm. So the executable file wouldn't have worked as an executable this entire time? That's something that will need better testing because that should have been caught before.

BillyRuffian commented 5 years ago

Well, your test suite was flagging it on my build. šŸ§ Iā€™m using a Mac so maybe a platform thing?

enkessler commented 5 years ago

I was thinking that it wasn't caught because the relevant test uses

When(/^the following command is executed:$/) do |command|
  command = "bundle exec ruby #{@executable_directory}/#{command}"

  @output = `#{command}`
end

and so it is being used as a regular Ruby script during testing instead of as a true executable. It's odd that it failed for you 'as is' because CI already includes Linux, OSX, and Windows. So if it were just a matter of where it ran then I would expect it to have failed earlier.

enkessler commented 5 years ago

Nope, it seems to work fine.

https://travis-ci.org/enkessler/cuke_linter/builds/535090563

Now I'm curious how you were trying to use it.

BillyRuffian commented 5 years ago
[dev] Ā» bundle exec rake cuke_linter:test_everything
...................F................................................................................................................................................................

Failures:

  1) the gem validates cleanly
     Failure/Error: expect(mock_ui.error).to_not match(/warn/i)

       expected "WARNING:  exe/cuke_linter is not executable\nWARNING:  See http://guides.rubygems.org/specification-reference/ for help\n" not to match /warn/i
       Diff:
       @@ -1,2 +1,3 @@
       -/warn/i
       +WARNING:  exe/cuke_linter is not executable
       +WARNING:  See http://guides.rubygems.org/specification-reference/ for help

     # ./testing/rspec/spec/unit/cuke_linter_unit_spec.rb:16:in `block (2 levels) in <top (required)>'

Finished in 0.13763 seconds (files took 0.19075 seconds to load)
180 examples, 1 failure

Bundler 2.0.1, Ruby 2.6.3.

enkessler commented 5 years ago

Oh, my bad. I was thinking that you were trying to use the executable yourself, not that the suite was failing. Hmm. We're testing against Ruby 2.6 in CI and it's working fine. I slapped 2.6.3 on my Windows machine and it's still fine with Bundler 2.0.1. I know that's not your exact setup but I'm not sure why Bundler version and patch level Ruby version differences would impact how the gem validation test works. I suppose that Gem::MockGemUi could have changed its behavior.

If you drop back to a version of Ruby that CI is testing against, do you still encounter that test failure?

enkessler commented 5 years ago

In any case, it's no reason to not take an otherwise perfectly good PR. I'll just revert that one commit.

BillyRuffian commented 5 years ago

Well now, here's an interesting thing...

[devā—] Ā» git reset --hard
[devā—] Ā» ls -l exe/cuke_linter
-rw-r--r--  1 ---  ---  61 21 May 09:55 exe/cuke_linter
[dev] Ā» bundle install
...
-rwxr-xr-x  1 ---  ---  61 23 May 09:27 exe/cuke_linter

Bundler sets the executable flag during an install so it's never going to fail in CI.