Avoid showing "@ rb_sysopen" noise for Ruby 2.4. #1080

junaruga · 2017-02-15T00:21:04Z

Summary

This fixes #1071.

Details

This issue happens for only Ruby 2.4.0.

Motivation and Context

Want to pass tests.

How Has This Been Tested?

$ ruby -v
ruby 2.4.0p0 (2016-12-24 revision 57164) [x86_64-linux]

$ bundle exec rake spec
...
568 examples, 0 failures

$ bundle exec rake cucumber
...
207 scenarios (17 failed, 190 passed)
1217 steps (17 failed, 1200 passed)

There are still failed tests. But they will be fixed by #1079 .
The rb_sysopen message was disappeared.

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

I've added tests for my code
My change requires a change to the documentation.
I have updated the documentation accordingly.

nodo · 2017-02-15T15:17:42Z

lib/cucumber/runtime.rb

-    include FixRuby21Bug9285 if Cucumber::RUBY_2_1 || Cucumber::RUBY_2_2 || Cucumber::RUBY_2_3
+  class FeatureFolderNotFoundException < Exception
+    attr :path
+    def initialize(path)


Do we need a getter for path variable here?

@nodo I thought previously FeatureFolderNotFoundException had the getter for path variable, because it had extended FileException. But as we do not use the getter right now, removing it might be better. I am going to remove it.

nodo · 2017-02-15T15:22:54Z

lib/cucumber/runtime.rb

+    attr :path
+    def initialize(path)
+      super('No such file or directory - features')
+      @path = path


Instead of calling super, I would suggest to override message method and output the path related to the error. E.g. "Feature folder not found at #{@path}" or similar. If we follow this path I would also use File.expand_path(path) for clarity.

@nodo ok I will follow your way.

nodo · 2017-02-15T15:23:22Z

lib/cucumber/platform.rb

@@ -14,6 +14,7 @@ module Cucumber
    RUBY_2_2      = RUBY_VERSION =~ /^2\.2/
    RUBY_2_1      = RUBY_VERSION =~ /^2\.1/
    RUBY_2_3      = RUBY_VERSION =~ /^2\.3/
+    RUBY_2_4      = RUBY_VERSION =~ /^2\.4/



Do we need these constants at all? /cc @mattwynne @brasmusson

I think this PR removes the only usage, so I'd say we can remove them.

Thanks @mattwynne 👍

@nodo ok I will remove them 👍

this PR removes the only usage,

I am still not sure above point. actually this PR can remove these constants?
or this PR should not touch these constants?

The constants RUBY_2_* are used in gem_tasks/cucumber.rake.

$ grep -r RUBY_2_ * gem_tasks/cucumber.rake: elsif Cucumber::RUBY_2_0 gem_tasks/cucumber.rake: elsif Cucumber::RUBY_2_1 lib/cucumber/runtime.rb: include FixRuby21Bug9285 if Cucumber::RUBY_2_1 || Cucumber::RUBY_2_2 || Cucumber::RUBY_2_3 lib/cucumber/platform.rb: RUBY_2_2 = RUBY_VERSION =~ /^2\.2/ lib/cucumber/platform.rb: RUBY_2_1 = RUBY_VERSION =~ /^2\.1/ lib/cucumber/platform.rb: RUBY_2_3 = RUBY_VERSION =~ /^2\.3/

nodo · 2017-02-15T15:24:27Z

Thanks @junaruga - just added a couple of comments.

junaruga · 2017-02-15T17:03:41Z

@nodo ok, let me check your coments later. Thanks.

junaruga · 2017-02-16T14:08:11Z

@nodo wait a moment. I will check it today or tomorrow as right now I am busy.

nodo · 2017-02-16T14:59:57Z

@junaruga no problem at all. Thanks for working on this!

junaruga · 2017-02-18T13:26:28Z

@nodo I modified my code and did rebase after review.
You can check the result on Travis too.

nodo · 2017-02-20T16:22:31Z

features/docs/getting_started.feature

@@ -9,7 +9,7 @@ Feature: Getting started
    When I run `cucumber`
    Then it should fail with:
      """
-      No such file or directory - features. You can use `cucumber --init` to get started.
+      Feature folder not found at
      """


I think this message should be more detailed. Some possible solutions:

Use Aruba.config.root_directory and somehow use it in the string

Change the exception to report a relative path and not a absolute path (so that here we can write tmp/features

Change the exception to not report the path

Sorry for this, I haven't notice this test before. However, I think it's important to keep it as informative as possible.

What do you think?

@nodo I like kind of 2. "kind of 2" means when I removed "File.expand_path", I did not need to add `tmp/features' to features/docs/getting_started.feature. Check my additional commit, I am going to push it right now.

I prefer File.expand_path.
But I like you would add File.expand_path by yourself later, if you need it.
Because right now I want to pass all the tests on Ruby 2.4.0.

voxik · 2017-02-23T10:02:30Z

The error message "Feature folder not found at features.", which is default output with this patch applied, is more confusing then helpful IMO. It is hardly obvious that the "features" refers to folder. If there was the dash as there used to be, that would be a bit better probably ....

This fixes cucumber#1071.

junaruga · 2017-02-23T15:08:42Z

@nodo I changed and did rebase of my code. I do not change the message by myself on this PR.
I did not make it worse from current state for this PR. Maybe.

I like you will merge this, and ask you to improve the message by yourself after this PR as you like.

junaruga · 2017-02-23T18:26:55Z

@nodo I can tell you other options too.

You can send your additional code to my repo https://github.com/junaruga/cucumber-ruby branch: feature/fix-rb_sysopen-on-ruby-2.4 as a pull-request. I am going to accept your any code.
You can close this pull-request. and you can fix it by yourself.

It does not matter for me. You can make a choice.

nodo · 2017-02-23T20:38:47Z

@junaruga sorry for not being responsive. I will check this PR as soon as possible. Thanks for your patience and for your work. Your changes looks good to me, I will just check the message as suggested by @voxik.

junaruga · 2017-02-26T20:26:43Z

@nodo Thank you!!

lock · 2018-10-25T02:07:41Z

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

nodo reviewed Feb 15, 2017

View reviewed changes

junaruga force-pushed the feature/fix-rb_sysopen-on-ruby-2.4 branch from a0b1484 to a0ce155 Compare February 18, 2017 13:15

junaruga force-pushed the feature/fix-rb_sysopen-on-ruby-2.4 branch from a0ce155 to c323c79 Compare February 18, 2017 13:32

nodo reviewed Feb 20, 2017

View reviewed changes

Avoid showing "@ rb_sysopen" noise for Ruby 2.4.

6f4be0a

This fixes cucumber#1071.

junaruga force-pushed the feature/fix-rb_sysopen-on-ruby-2.4 branch from 40b6b10 to 6f4be0a Compare February 23, 2017 15:01

nodo approved these changes Feb 26, 2017

View reviewed changes

nodo merged commit 552c6e6 into cucumber:master Feb 26, 2017

junaruga mentioned this pull request Feb 26, 2017

rake cucumber is failed on Ruby 2.4 #1070

Closed

lock bot locked as resolved and limited conversation to collaborators Oct 25, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Avoid showing "@ rb_sysopen" noise for Ruby 2.4. #1080

Avoid showing "@ rb_sysopen" noise for Ruby 2.4. #1080

junaruga commented Feb 15, 2017 •

edited

nodo Feb 15, 2017

junaruga Feb 18, 2017

nodo Feb 15, 2017

junaruga Feb 18, 2017

nodo Feb 15, 2017

mattwynne Feb 15, 2017

nodo Feb 16, 2017

junaruga Feb 18, 2017

junaruga Feb 18, 2017

nodo commented Feb 15, 2017

junaruga commented Feb 15, 2017

junaruga commented Feb 16, 2017

nodo commented Feb 16, 2017

junaruga commented Feb 18, 2017 •

edited

nodo Feb 20, 2017

junaruga Feb 20, 2017

voxik commented Feb 23, 2017

junaruga commented Feb 23, 2017

junaruga commented Feb 23, 2017 •

edited

nodo commented Feb 23, 2017

junaruga commented Feb 26, 2017

lock bot commented Oct 25, 2018

Avoid showing "@ rb_sysopen" noise for Ruby 2.4. #1080

Avoid showing "@ rb_sysopen" noise for Ruby 2.4. #1080

Conversation

junaruga commented Feb 15, 2017 • edited

Summary

Details

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nodo commented Feb 15, 2017

junaruga commented Feb 15, 2017

junaruga commented Feb 16, 2017

nodo commented Feb 16, 2017

junaruga commented Feb 18, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

voxik commented Feb 23, 2017

junaruga commented Feb 23, 2017

junaruga commented Feb 23, 2017 • edited

nodo commented Feb 23, 2017

junaruga commented Feb 26, 2017

lock bot commented Oct 25, 2018

junaruga commented Feb 15, 2017 •

edited

junaruga commented Feb 18, 2017 •

edited

junaruga commented Feb 23, 2017 •

edited