1281570 - Provide recommendations to fix typos when using `mach try`

Status: REOPENED

(Developer Infrastructure :: Try, defect)

Description

[Jared Wein [:jaws] (please needinfo? me)]

Since bug 1258451 we now will validate whether some of the options are correct, but we print out a big list of all of the options.

It would be better if we only printed options that were close to what was requested.

For example:

./mach try --no-push -p win -u mochitest
Invalid choice 'win'. Did you mean ['win32', 'win64]?
Invalid choice 'mochitest'. Did you mean ['mochitests', 'mochitest-1', 'mochitest-2', 'mochitest-3', 'mochitest-4', 'mochitest-5', 'mochitest-6', 'mochitest-7', 'mochitest-8', 'mochitest-9']?

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch v1.1

Comment 3

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8764372 [details] [diff] [review]
Patch v1.1

Review of attachment 8764372 [details] [diff] [review]:
-----------------------------------------------------------------

::: testing/tools/autotry/autotry.py
@@ +14,5 @@
>  
>  import ConfigParser
>  
> +# Implementation based on pseudo-code from Wikipedia,
> +# https://en.wikipedia.org/wiki/Levenshtein_distance#Iterative_with_two_matrix_rows

I see we use difflib from the Python standard library in other places around the tree, would a function from that library be appropriate here?

@@ +58,5 @@
> +            if distance < 3L:
> +                corrections.append(choice)
> +        if len(corrections) > 0:
> +            valid = False
> +            print 'Invalid choice {v!r}. Did you mean: {c!r}?'.format(v=value, c=corrections)

This has an interesting effect: near misses are rejected, but everything else is accepted.

This actually seems like a decent behavior in this case (although I guess it would be problematic in the case we did something like add chunks).

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch v2

Thanks, this is much better.

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch v2.1

Actually, since the v1 and v2 patch allowed non-similar errors to pass through it regressed bug 1277127.

This version of the patch will now report when there are no similar matches and ask the user if they are sure they want to proceed.

I also fixed the placement of the 'under development' warning to appear before any parsing validation has run.

Comment 6

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8764594 [details] [diff] [review]
Patch v2.1

Review of attachment 8764594 [details] [diff] [review]:
-----------------------------------------------------------------

::: testing/tools/autotry/autotry.py
@@ +20,4 @@
>      for value in values:
> +        corrections = difflib.get_close_matches(value, choices)
> +        if len(corrections) == 0:
> +            print 'Potentially invalid choice {v!r}. Requested value nor similar values are not found in list of possible values.'.format(v=value)

This sentence read a little easier as "Neither the requested value nor a similar value..."

@@ +28,5 @@
> +        elif corrections[0] == value:
> +            continue
> +        else:
> +            valid = False
> +            print 'Invalid choice {v!r}. Some suggestions (limit three): {c!r}?'.format(v=value, c=corrections)

Do you need to pass 3 to get_close_matches to actually limit the results?

Comment 7

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Chris Manchester (:chmanchester) from comment #6)
> > +        elif corrections[0] == value:
> > +            continue
> > +        else:
> > +            valid = False
> > +            print 'Invalid choice {v!r}. Some suggestions (limit three): {c!r}?'.format(v=value, c=corrections)
> 
> Do you need to pass 3 to get_close_matches to actually limit the results?

No, because 3 is the default value used by get_close_matches. https://docs.python.org/2/library/difflib.html

Comment 8

[Jared Wein [:jaws] (please needinfo? me)]

https://hg.mozilla.org/integration/fx-team/rev/efec1285a2a18a1fef1c6242b22d72638e088330
Bug 1281570 - Provide recommendations to fix typos when using 'mach try'. r=chmanchester

Comment 9

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/efec1285a2a1

Comment 10

[Chris Manchester (limited bugmail, email directly)]

Backed this out as part of the backout for bug 1258451 in

https://hg.mozilla.org/integration/mozilla-inbound/rev/ea448e55b6426a2afc24e29c38e66813a6dc0c70