<div dir="ltr">Hi Kristof,<br><br><div>I was thinking about this today, and I think the easiest way to work this from a UI perspective would be to just provide checkboxes next to the machine in the overview table - toggling these boxes would toggle the visibility of results from that run on-the-fly (via javascript), similar to how the new test regexp filter box works in the v4_runs page.</div><div><br></div><div>What do you think?</div><div><br></div><div>Cheers,</div><div><br></div><div>James</div></div><br><div class="gmail_quote"><div dir="ltr">On Thu, 4 Jun 2015 at 12:09 Kristof Beyls <<a href="mailto:kristof.beyls@arm.com">kristof.beyls@arm.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Thanks for the review, I've committed the test refactorings as<br>
r238296-r238300. You're right, it makes sense to filter machines<br>
based on a pattern rather than on the full name. I've considered<br>
just a sub-string search, matching with a glob or matching with<br>
a python regex. I feel that matching with a python regex is the<br>
best solution as it allows the most flexible filtering.<br>
<br>
I went for "?filter-machine-regex=" to enable filtering on the<br>
daily report webui and "--filter_machine_regex" to enable<br>
filtering when using the lnt send-daily-report command.<br>
<br>
See attached patch for details. Does this look OK to you?<br>
<br>
Kristof<br>
<br>
> -----Original Message-----<br>
> From: Chris Matthews [mailto:<a href="mailto:chris.matthews@apple.com" target="_blank">chris.matthews@apple.com</a>]<br>
> Sent: 26 May 2015 19:28<br>
> To: Kristof Beyls<br>
> Cc: llvm-commits<br>
> Subject: Re: [PATCH][LNT] Add ability to filter machines on daily report<br>
> page + refactor LNT regression tests to enable properly testing it.<br>
><br>
> I completely agree with this testing strategy. This is a much better way<br>
> to test.<br>
><br>
> I'm fine with an explicit machine list, but I wonder if a more useful<br>
> way to do this is to have a machine name filter? That way queries like<br>
> "?filter-machine=AArch64&" could be possible along with an explicit<br>
> "?filter-machine=lvm-juno-lnt-perf__LNT-AArch64-A53-<br>
> O3__clang_DEV__aarch64&".  Just substring matching on the machine name.<br>
> That would make it very easy to grab classes of machines.<br>
><br>
> Please add period to end of comment on patch 6 line 46.<br>
><br>
> Could you update the command line daily report tool<br>
> (lnt/lnttool/main.py:304) to also have this functioanltiy? I think the<br>
> change should be really simple.<br>
><br>
> Otherwise looks good!<br>
><br>
> > On May 26, 2015, at 3:41 AM, Kristof Beyls <<a href="mailto:kristof.beyls@arm.com" target="_blank">kristof.beyls@arm.com</a>><br>
> wrote:<br>
> ><br>
> > Hi Chris,<br>
> ><br>
> > I'd like to be able to filter on the daily report page, e.g.<br>
> > <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_perf_db-5Fdefault_v4_nts_daily-5Freport_2015_5_21-3Fnum-5Fdays&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=486v8RsjLZXtBXHgJt6CR_k6f0_2J15UCZM1jTkDQu4&s=tFGj76-FZL3M2x1sCDkHThDAdV7YuHgLh3LRbTWt7QU&e=" target="_blank">http://llvm.org/perf/db_default/v4/nts/daily_report/2015/5/21?num_days</a><br>
> > =7, so that it only shows data for a subset of the builders. The<br>
> > previous link shows many results coming from the penryn builders,<br>
> > which make it hard to just investigate the results coming from the<br>
> > juno board.<br>
> ><br>
> > 0006-Enable-filtering-of-machines-shown-on-LNT-daily-repo.patch adds<br>
> > support for this; the user visible behaviour I went for is adding<br>
> > machines={comma-separated list of machine names} to the query string<br>
> > of the url, e.g.<br>
> > <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_perf_db-5Fdefault_v4_nts_daily-5Freport_2015_5_21-3Fnum-5Fdays&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=486v8RsjLZXtBXHgJt6CR_k6f0_2J15UCZM1jTkDQu4&s=tFGj76-FZL3M2x1sCDkHThDAdV7YuHgLh3LRbTWt7QU&e=" target="_blank">http://llvm.org/perf/db_default/v4/nts/daily_report/2015/5/21?num_days</a><br>
> > =7&mac<br>
> > hines=llvm-juno-lnt-perf__LNT-AArch64-A53-O3__clang_DEV__aarch64.<br>
> ><br>
> > To be able to add a proper regression test/unit test for this, I also<br>
> > had to refactor the LNT regression tests a bit, which is what the<br>
> > other patches contain. In a number of steps, they transform the tests<br>
> > using tests/SharedInputs/SmallInstance/lnt.db to using a database<br>
> > constructed from human-readable SQL statements, creating the sqlite<br>
> > file on the fly during test execution. This enables adding per-test<br>
> > records to the database:<br>
> ><br>
> > - 0001-For-the-regression-tests-make-temporary-LNT-db-insta.patch<br>
> > - 0002-Replace-binary-test-lnt.db-with-human-readable-text-.patch<br>
> > - 0003-replace-hexadecimal-encoding-of-blobs-with-textual-r.patch<br>
> > - 0004-remove-most-of-the-unnecessary-insert-statement-from.patch<br>
> > - 0005-Enable-adding-test-specific-records-to-test-database.patch<br>
> ><br>
> > My hope is that this refactoring enables creating new functionality<br>
> > for the LNT ui in a more Test-Driven Development way, which should<br>
> > make it easier for more people to contribute to LNT & to reduce the<br>
> > likelihood of regressions in LNT.<br>
> ><br>
> > What do you think?<br>
> > Is the refactoring of the regression tests a step in the right<br>
> direction?<br>
> > Any objections to adding the machine= filter to the daily report page?<br>
> ><br>
> > Thanks,<br>
> ><br>
> > Kristof<br>
> > <0001-For-the-regression-tests-make-temporary-LNT-db-insta.patch><0002<br>
> > -Replace-binary-test-lnt.db-with-human-readable-text-.patch><0003-repl<br>
> > ace-hexadecimal-encoding-of-blobs-with-textual-r.patch><0004-remove-mo<br>
> > st-of-the-unnecessary-insert-statement-from.patch><0005-Enable-adding-<br>
> > test-specific-records-to-test-database.patch><0006-Enable-filtering-of<br>
> > -machines-shown-on-LNT-daily-repo.patch><br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>