Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

Model attribute equality step definition doesn't handle string values correctly

#21

I'm trying to use the step defined here. However, it's capturing the double quotes that surround the string, leading to a failure. Here's my cucumber output:

  Scenario: Edit a geography with invalid values                   # features/manage_geographies.feature:36
    Given a geography exists with name: "Los Angeles", state: "CA" # features/step_definitions/pickle_steps.rb:4
    And I am logged in as an admin                                 # features/step_definitions/authentication_steps.rb:13
    And I am on the homepage                                       # features/step_definitions/web_steps.rb:18
    When I follow these links: "Admin", "Geographies", "Edit"      # features/step_definitions/additional_web_steps.rb:5
    And I fill in "Name" with ""                                   # features/step_definitions/web_steps.rb:38
    And I press "Save"                                             # features/step_definitions/web_steps.rb:26
    Then I should see the record validation error message          # features/step_definitions/additional_web_steps.rb:13
    And that geography's name should be "Los Angeles"              # features/step_definitions/pickle_steps.rb:75
      
      expected "\"Los Angeles\""
           got "Los Angeles"
      
      (compared using eql?)
       (Spec::Expectations::ExpectationNotMetError)
      ./features/step_definitions/pickle_fixes.rb:16:in `send'
      ./features/step_definitions/pickle_fixes.rb:16:in `/^((?:(?:I|myself|me|my)|(?:(?:a|an|another|the|that) )?(?:(?:(?:(?:first|last|(?:\d+(?:st|nd|rd|th))) )?(?:address|slug|promotion[_ ]view|business|encoded[_ ]email|active[_ ]admin|promotion|customer[_ ]information|merchant[_ ]payment|promotion[_ ]transaction|delayed[_ ]job|admin|active[_ ]user|voucher|business[_ ]link|promotion[_ ]referrer|geography|promotion[_ ]user|merchant|business[_ ]category|user))|(?:(?:address|slug|promotion[_ ]view|business|encoded[_ ]email|active[_ ]admin|promotion|customer[_ ]information|merchant[_ ]payment|promotion[_ ]transaction|delayed[_ ]job|admin|active[_ ]user|voucher|business[_ ]link|promotion[_ ]referrer|geography|promotion[_ ]user|merchant|business[_ ]category|user)(?::? "(?:[^\"]|\.)*")))))'s (\w+) (should(?: not)?) be ((?:"(?:[^\"]|\.)*"|nil|true|false|[+-]?\d+(?:\.\d+)?))$/'
      features/manage_geographies.feature:44:in `And that geography's name should be "Los Angeles"'

I'm willing to take a stab at fixing this and submit my fix, but I wanted to get feedback about the best way to go about this. I see two ways to fix this:

  1. By fixing #capture_value so that it doesn't include the quotes in the capture.
  2. By stripping the quotes from the value before checking equality, using something like expected = expected.gsub(/^\"/, '').gsub(/\"$/, '').

I like the 1st solution better overall, but it's complicated by how pickle implements the #capture_value. The problem is that the basic regex is defined by #match_value, which uses quotes to tell that it's a string. Then this dynamic code surrounds it with the capturing parenthesis when it defines #capture_value. I don't see a way to fix the regular expression to not capture the double quotes unless we define the entire #capture_value method rather than defining it dynamically.

Any thoughts on how this should be fixed?

Reported by Myron Marston · March 12th, 2010 @ 07:13 AM

State: new
Milestone: none
Assigned to: Ian White Ian White

Activity

  1. Ian White
    Ian White

    Hi Myron,

    Thanks for taking the time to report this issue, and sorry it has taken so long for a reply (new father and all that)

    I also prefer (1)

    The dynamic code you refer to does get in the way here, and it will probably go soon. But in the meantime you could simply redefine capture_value after that fragment to make the better capture_value method.

    April 5th, 2010 @ 08:53 PM

  2. Myron Marston
    Myron Marston

    First off, congrats on fatherhood! No worries on the delay--you had more important things going on in your life :).

    I attempted to fix the regex, but I haven't had success. I could be overlooking something, but I think we need to use a positive lookbehind assertion to do this properly...but Ruby 1.8.6's regex engine doesn't support lookbehind assertions.

    The problem is that we have to ensure the double quotes are surrounding the string, or else the regex is too permissive. I tried capturing the string case separately from the other cases (i.e. nil, true, false, number), but then they are separate capture groups, and it causes cucumber to yield them as separate arguments to your step definition block. To do this properly, the regex has to ensure that the string is surrounded by double quotes from within the capturing parenthesis, without including the parenthesis in the capture.

    I'm no regex expert (but I would consider myself generally proficient), and as far as I know, lookahead and lookbehind assertions are the only way to do this. (I'd love to be proven wrong though...) I think we're out of luck until ruby 1.9 has sufficient market share that you decide you no longer need to support 1.8.6.

    Anyhow, I went ahead and fixed this with the other solution here. Definitely not as elegant, but it works.

    April 6th, 2010 @ 05:04 AM

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.