#21 new
Myron Marston

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

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

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?

Comments and changes to this ticket

  • Ian White

    Ian White April 5th, 2010 @ 08:53 PM

    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.

  • Myron Marston

    Myron Marston April 6th, 2010 @ 05:04 AM

    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.

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.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile ยป

Assign tickets to <b>Ian White</b>, so I get notified

People watching this ticket

Pages