Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Levenshtein Distance incorrectly chosen #187

Closed
Ashthos opened this issue Jan 5, 2016 · 2 comments
Closed

Levenshtein Distance incorrectly chosen #187

Ashthos opened this issue Jan 5, 2016 · 2 comments

Comments

@Ashthos
Copy link

Ashthos commented Jan 5, 2016

I have discovered a bug with the Levenshtein calculation in 0.16.2, versions prior this work correctly.

The issue occurs when two steps appear in the same library and appears to be sensitive to the ordering of the steps within the library.

The following feature and steps reproduce the bug.

Feature: Yadda works as expected
  As a consumer of the Yadda framework
  I want it to behave as expected
  So that I do not introduce bugs when upgrading.

Scenario: Levenshtein Distance is calculated correctly
  Given I view the "Ignored" with properties "?Hello=1"
  Then the "Levenshtein-Test" context value is "With Properties"

Scenario: Levenshtein Distance is calculated correctly - Two
  Given I attempt to view the "Ignored"
  Then the "Levenshtein-Test" context value is "Attempting"

Scenario: Levenshtein Distance is calculated correctly - Three
  Given I view the "Ignored"
  Then the "Levenshtein-Test" context value is "Attempting"

Note: Ordering of the step definitions is important within the library.

    .given('I (attempt to )?view the "(.*)"', function (attempting, pageName, next){
        this.context['Levenshtein-Test'] = 'Attempting';
        next();
    })

    .given('I view the "(.*)" with properties "(.*)"', function (pageName, properties, next) {
        this.context['Levenshtein-Test'] = 'With Properties';
        next();
    })

    .then('the "(.*)" context value is "(.*)"', function (variableName, expectedValue, next){
        expect(this.context[variableName]).toBe(expectedValue);
        next();
    })

0.15.3 - 0.16.1 correctly chooses the second step definition in the 1st scenario.
0.16.2 incorrectly chooses the first step definition in the 1st scenario.

If the order of the steps is swapped (i.e. 'with properties' appears first) then the tests will pass in all versions (0.15.3 - 0.16.2). However in our suite of tests we have a number of situations where fixing for one breaks others and cannot use this solution.

@cressie176
Copy link
Member

Hi - thanks for reporting. It's probably not the Levenshtein distance algorithm but another change related to step selection.

My intention was when two steps have an identical Levenshtein's distance, to prefer one if it came from the same library as the previous step. If both steps have identical Levenshtein's distance and come from the same library then Yadda should report an ambiguous step.

I'll take a look this evening. This was the only change in 0.16.2 so for now if you fix to 0.16.1 you should be fine.

@cressie176
Copy link
Member

0.16.3 released. I tried it with your feature and it passed ok. Sorry for the inconvenience and thanks for taking the trouble to put together such a comprehensive report.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants