Closed
Bug 1384593
Opened 7 years ago
Closed 7 years ago
Create a fuzzy try selector
Categories
(Testing :: General, enhancement)
Testing
General
Tracking
(firefox56 fixed)
RESOLVED
FIXED
mozilla56
Tracking | Status | |
---|---|---|
firefox56 | --- | fixed |
People
(Reporter: ahal, Assigned: ahal)
References
Details
Attachments
(2 files)
Create a try selector based on fzf: https://github.com/junegunn/fzf
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 3•7 years ago
|
||
Here's a demo of this in action: https://ahal.ca/static/vid//blog/2017/mach-fuzzy.mp4
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 6•7 years ago
|
||
If you want to try this out locally, you'll need the latest version-control-tools: ./mach mercurial-setup --update I tested this on Windows, and with git-cinnabar.
Comment 7•7 years ago
|
||
mozreview-review |
Comment on attachment 8890488 [details] Bug 1384593 - Abstract version control functionality out of syntax.py to vcs.py, https://reviewboard.mozilla.org/r/161632/#review168422 What a nice clean up. Land this as long as you intended to have a constructor to set self.root to root. ::: tools/tryselect/vcs.py:43 (Diff revision 3) > +class VCSHelper(object): > + """A abstract base VCS helper that detects hg or git""" > + __metaclass__ = ABCMeta > + > + def __init__(self, root): > + self.root = root What is self.root for?
Attachment #8890488 -
Flags: review?(armenzg) → review+
Assignee | ||
Comment 8•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8890488 [details] Bug 1384593 - Abstract version control functionality out of syntax.py to vcs.py, https://reviewboard.mozilla.org/r/161632/#review168422 > What is self.root for? Good catch! That got copied from: https://dxr.mozilla.org/mozilla-central/source/python/mozlint/mozlint/vcs.py#13 and isn't being used here. I'm still tempted to leave it in though, just because we're already spending time running the `hg` or `git` command to detect the root and it could be useful in the future. That said if you prefer I remove it, I'd be happy to do so.
Comment 9•7 years ago
|
||
mozreview-review |
Comment on attachment 8892076 [details] Bug 1384593 - Add an fzf based fuzzy try selector, https://reviewboard.mozilla.org/r/163092/#review168426 No issues preventing landing. Pick and choose what needs addressing and what not. Fantastic job. Looking forward to using this. ::: tools/tryselect/mach_commands.py:71 (Diff revision 1) > kwargs = vars(parser.parse_args(args)) > return self._mach_context.commands.dispatch( > 'try', subcommand='syntax', context=self._mach_context, **kwargs) > > @SubCommand('try', > + 'fuzzy', Is "fuzzy" going to be the final name? For me when I think of this topic I'm inclined to think of what I want to happen ("select tasks" or "define what to schedule") rather than the matching algorithm ('fuzzy finding'). *I* would have named it 'task-selection'. I'm not strongly tied to my opinion. I might be thinking this too much because English is not my first language. ::: tools/tryselect/selectors/fuzzy.py:176 (Diff revision 1) > +def run_fuzzy_try(update): > + fzf = fzf_bootstrap(update) > + > + if not fzf: > + print(FZF_NOT_FOUND) > + return No need for sys.exit()? ::: tools/tryselect/tasks.py:25 (Diff revision 1) > +def invalidate(cache): > + if not os.path.isfile(cache): > + return > + > + tc_dir = os.path.join(build.topsrcdir, 'taskcluster') > + tmod = max(os.path.getmtime(os.path.join(tc_dir, p)) for p, _ in FileFinder(tc_dir)) Clever. It would be cool if our checkouts had a faster way to determine if anything has changed since last time. Could there be an artifact we can fetch from the gecko decision task? Or only check the files that have change since we branched? This might be a candidate for a follow up project.
Attachment #8892076 -
Flags: review?(armenzg) → review+
Assignee | ||
Comment 10•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8892076 [details] Bug 1384593 - Add an fzf based fuzzy try selector, https://reviewboard.mozilla.org/r/163092/#review168426 Thanks for the reviews, I'm looking forward to getting this out there for more feedback too! > Is "fuzzy" going to be the final name? > > For me when I think of this topic I'm inclined to think of what I want to happen ("select tasks" or "define what to schedule") rather than the matching algorithm ('fuzzy finding'). > > *I* would have named it 'task-selection'. I'm not strongly tied to my opinion. > > I might be thinking this too much because English is not my first language. That's a good point, but I think the name needs to describe *how* the tasks are being selected. That's because all of the `try selectors` (right now just `syntax` and `fuzzy`, but in my vision there could be 4-5 of them eventually), are trying to accomplish that goal you stated of "defining what to schedule". E.g `task-selection` could refer to either the fuzzy finder or try syntax. And it will get more confusing if we add a curses selector + regex/glob selector + a mozbuild.testing.TestResolver based selector. They're all trying to select tasks in various ways. > No need for sys.exit()? Hm, yeah I guess my use of sys.exit() vs return is pretty inconsistent. It was mostly to avoid checking return codes. I'll see if I can clean it up a bit. > Clever. > > It would be cool if our checkouts had a faster way to determine if anything has changed since last time. > > Could there be an artifact we can fetch from the gecko decision task? > Or only check the files that have change since we branched? > > This might be a candidate for a follow up project. While this is running on a lot of files, it's actually surprisingly fast. I'm not even sure that downloading an artifact would be an improvement. We could do this using build backends (i.e re-use the thing that the makefiles do to detect if the build configuration is out of date), but I think that means we'd need some kind of taskcluster -> build integration. It would be a pretty big project.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 13•7 years ago
|
||
Pushed by ahalberstadt@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ac8b6cc020b9 Abstract version control functionality out of syntax.py to vcs.py, r=armenzg https://hg.mozilla.org/integration/autoland/rev/9d617ec5226c Add an fzf based fuzzy try selector, r=armenzg
Comment 14•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/ac8b6cc020b9 https://hg.mozilla.org/mozilla-central/rev/9d617ec5226c
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in
before you can comment on or make changes to this bug.
Description
•