Last modified: 2013-07-02 13:07:33 UTC
Cucumber step definition files[1] are full of code that causes Ruby to output warnings. Example: $ ruby -cw features/step_definitions/aftv5_steps.rb features/step_definitions/aftv5_steps.rb:1: warning: ambiguous first argument; put parentheses or even spaces ... Syntax OK Cucumber fixed the problem in version 1.2.2[2][3] but we still have a lot of code that was created using earlier versions. The fix is easy. Example: Replace Given /^I am at an AFTv5 page$/ do visit AFTv5Page end With Given(/^I am at an AFTv5 page$/) do visit AFTv5Page end (Only parentheses are added.) 1: https://github.com/wikimedia/qa-browsertests/tree/master/features/step_definitions 2: https://github.com/cucumber/cucumber/blob/master/History.md#122 3: https://github.com/cucumber/cucumber/issues/328
I came upon this by chance, and let me just note that as a hobbyist Ruby programmer I personally consider the -w warnings utterly useless and annoying. The parentheses parsing behavior is well-defined and IMO intuitive (unless you expect whitespace to be entirely insignificant) and the warnings should just be ignored if the code reads better this way.
(In reply to comment #1) > The parentheses parsing behavior is well-defined and IMO intuitive (unless > you expect whitespace to be entirely insignificant) To expand on this for posterity, as you yourself probably know what the issue is: The following are method calls, calling `meth` with an argument of `/regex#/`: meth(/regex#/) meth (/regex#/) meth /regex#/ The following are divisions, dividing `meth` by `regex` (which as usual can be methods or variables; the rest of the line is a comment): meth / regex#/ meth/regex#/ So a method call is only used when either explicitly enforced by the parentheses or when the whitespace on two sides of '/' is not balanced. This seems pretty intuitive to me. Similar warnings also pop up all the time when using '*' without parentheses all over (multiplication vs unary "splat"). They can also be triggered by unary minus or plus.
I do not really care whether we use parentheses around regular expressions or not, but Cucumber (since version 1.2.2) adds parentheses, so we ended up with two styles. I do care about having a coding convention, which ever we pick. Since Cucumber adds parentheses, my vote is to convert all code to use them, instead of making sure to remove them every time when we copy/paste code from Cucumber output.
Sure, that makes sense. This was just my two cents :)
Thank you for your donation! :)
Related URL: https://gerrit.wikimedia.org/r/70045 (Gerrit Change Iceb703513d79f76d3b763c379a98f066822372f6)
Fixed in https://gerrit.wikimedia.org/r/#/c/70045
z@imac:~/project/wmf/browsertests(master)$ find . -name "*.rb" ./features/step_definitions/aftv5_steps.rb ./features/step_definitions/create_account_steps.rb ./features/step_definitions/file_steps.rb ./features/step_definitions/guided_tour_steps.rb ./features/step_definitions/login_steps.rb ./features/step_definitions/login_sul_steps.rb ./features/step_definitions/math_steps.rb ./features/step_definitions/page_edit_steps.rb ./features/step_definitions/page_steps.rb ./features/step_definitions/page_triage_steps.rb ./features/step_definitions/pdf_steps.rb ./features/step_definitions/preferences_appearance_steps.rb ./features/step_definitions/preferences_datetime_steps.rb ./features/step_definitions/print_export_menu_steps.rb ./features/step_definitions/search_steps.rb ./features/step_definitions/uls_accept_language_steps.rb ./features/step_definitions/uls_cog_sidebar_user_steps.rb ./features/step_definitions/uls_ime_steps.rb ./features/step_definitions/uls_steps.rb ./features/step_definitions/upload_wizard_steps.rb ./features/step_definitions/visual_editor_steps.rb ./features/step_definitions/wikilove_steps.rb ./features/support/env.rb ./features/support/modules/interlanguage_module.rb ./features/support/modules/url_module.rb ./features/support/pages/aftv5_page.rb ./features/support/pages/article_page.rb ./features/support/pages/create_account_page.rb ./features/support/pages/describe_page.rb ./features/support/pages/does_not_exist_page.rb ./features/support/pages/edit_page.rb ./features/support/pages/edit_page_ie6.rb ./features/support/pages/file_does_not_exist_page.rb ./features/support/pages/interlanguage_page.rb ./features/support/pages/learn_page.rb ./features/support/pages/login_page.rb ./features/support/pages/main_page.rb ./features/support/pages/move_page.rb ./features/support/pages/no_interlanguage_page.rb ./features/support/pages/page_triage_page.rb ./features/support/pages/preferences_appearance_page.rb ./features/support/pages/preferences_date_time_page.rb ./features/support/pages/preferences_page.rb ./features/support/pages/random_page.rb ./features/support/pages/release_rights_page.rb ./features/support/pages/search_page.rb ./features/support/pages/search_results_page.rb ./features/support/pages/tour_page.rb ./features/support/pages/upload_page.rb ./features/support/pages/upload_wizard_page.rb ./features/support/pages/use_page.rb ./features/support/pages/visual_editor_page.rb ./features/support/pages/wikilove_page.rb ./features/support/sauce.rb z@imac:~/project/wmf/browsertests(master)$ find . -name "*.rb" | xargs -n1 ruby -cw Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK Syntax OK
