Re: [Piglit] [PATCH] framework/run.py: allow additional excluded tests on resume
OK, I'm abandoning this patch in favor of this one: [Piglit] [PATCH] framework/run.py: add option to not retry incomplete tests on resume Mike -Original Message- From: Dylan Baker [mailto:baker.dyla...@gmail.com] Sent: Tuesday, May 19, 2015 3:05 PM To: Mason, Michael W Cc: Daniel Vetter; piglit@lists.freedesktop.org Subject: Re: [Piglit] [PATCH] framework/run.py: allow additional excluded tests on resume On Tue, May 19, 2015 at 04:45:59PM +, Mason, Michael W wrote: How about something like this instead of my original patch? I think we should preserve the fact that some tests didn't complete. I'll submit this in a separate email if you all agree. --- a/framework/programs/run.py +++ b/framework/programs/run.py @@ -313,6 +313,9 @@ def resume(input_): type=argparse.FileType(r), help=Optionally specify a piglit config file to use. Default is piglit.conf) +parser.add_argument(-n, --no-retry, dest='no_retry', Drop dest down to a new line like the others please. +action='store_true', +help=Do not retry incomplete tests) args = parser.parse_args(input_) _disable_windows_exception_messages() @@ -342,7 +345,7 @@ def resume(input_): # Don't re-run tests that have already completed, incomplete status tests # have obviously not completed. for name, result in results.tests.iteritems(): -if result['result'] != 'incomplete': +if result['result'] != 'incomplete' or args.no_retry: It really doesn't matter but, should we check args.no_retry first? Since python is lazy I think it will be optimal in most cases. opts.exclude_tests.add(name) This looks reasonable to me. I left a couple of nits above. Dylan ___ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH] framework/run.py: allow additional excluded tests on resume
On Mon, May 18, 2015 at 05:12:17PM -0700, Dylan Baker wrote: I'm not sure this is a good idea, Ken Graunke and I talked this to death when I did the refactor that split the run and resume code into separate paths, and we agreed that changing the tests during a resume would make it impossible to reproduce a run, since the regex you pass might exclude tests that have already completed. That said, I'm opened to being convinced it is a good idea. With your patches to mark a test as incomplete, should we instead just have an option on resume to mark all incomplete tests as failed and not try to restart them? Or is that already what happens (tbh I haven't tried). -Daniel Dylan On Mon, May 18, 2015 at 02:29:58PM -0700, Mike Mason wrote: This patch allows additional tests to be excluded when resuming a test run. This is especially useful when a failing test causes a crash or reboot. Currently, that same test runs again when attempting to resume the test run, resulting in the same crash or reboot. Note that this does not change the exclude options saved at the start of the run. If a run is resumed multiple times, any additional excluded tests must be specified on the command line each time. Signed-off-by: Mike Mason michael.w.ma...@intel.com --- framework/programs/run.py | 14 +- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/framework/programs/run.py b/framework/programs/run.py index 6053074..89e4b69 100644 --- a/framework/programs/run.py +++ b/framework/programs/run.py @@ -308,6 +308,12 @@ def resume(input_): type=path.realpath, metavar=Results Path, help=Path to results folder) +parser.add_argument(-x, --exclude-tests, +default=[], +action=append, +metavar=regex, +help=Add tests to exclude + (can be used more than once)) parser.add_argument(-f, --config, dest=config_file, type=argparse.FileType(r), @@ -317,8 +323,14 @@ def resume(input_): _disable_windows_exception_messages() results = backends.load(args.results_path) + +# Additional tests to exclude from resumed run +exclude_filter = results.options['exclude_filter'] +if args.exclude_tests: +exclude_filter.extend(args.exclude_tests) + opts = core.Options(concurrent=results.options['concurrent'], -exclude_filter=results.options['exclude_filter'], +exclude_filter=exclude_filter, include_filter=results.options['filter'], execute=results.options['execute'], valgrind=results.options['valgrind'], -- 2.1.0 ___ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit ___ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH] framework/run.py: allow additional excluded tests on resume
-Original Message- From: Dylan Baker [mailto:baker.dyla...@gmail.com] Sent: Tuesday, May 19, 2015 9:42 AM To: Daniel Vetter Cc: Mason, Michael W; piglit@lists.freedesktop.org Subject: Re: [Piglit] [PATCH] framework/run.py: allow additional excluded tests on resume On Tue, May 19, 2015 at 12:06:54PM +0200, Daniel Vetter wrote: On Mon, May 18, 2015 at 05:12:17PM -0700, Dylan Baker wrote: I'm not sure this is a good idea, Ken Graunke and I talked this to death when I did the refactor that split the run and resume code into separate paths, and we agreed that changing the tests during a resume would make it impossible to reproduce a run, since the regex you pass might exclude tests that have already completed. That said, I'm opened to being convinced it is a good idea. With your patches to mark a test as incomplete, should we instead just have an option on resume to mark all incomplete tests as failed and not try to restart them? Or is that already what happens (tbh I haven't tried). -Daniel Currently they're always retried. It seems reasonable to me to either change the behavior or add a switch to change the behavior to either mark them as failed or just leave them as incomplete. How about something like this instead of my original patch? I think we should preserve the fact that some tests didn't complete. I'll submit this in a separate email if you all agree. --- a/framework/programs/run.py +++ b/framework/programs/run.py @@ -313,6 +313,9 @@ def resume(input_): type=argparse.FileType(r), help=Optionally specify a piglit config file to use. Default is piglit.conf) +parser.add_argument(-n, --no-retry, dest='no_retry', +action='store_true', +help=Do not retry incomplete tests) args = parser.parse_args(input_) _disable_windows_exception_messages() @@ -342,7 +345,7 @@ def resume(input_): # Don't re-run tests that have already completed, incomplete status tests # have obviously not completed. for name, result in results.tests.iteritems(): -if result['result'] != 'incomplete': +if result['result'] != 'incomplete' or args.no_retry: opts.exclude_tests.add(name) [snip] ___ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH] framework/run.py: allow additional excluded tests on resume
I'm not sure this is a good idea, Ken Graunke and I talked this to death when I did the refactor that split the run and resume code into separate paths, and we agreed that changing the tests during a resume would make it impossible to reproduce a run, since the regex you pass might exclude tests that have already completed. That said, I'm opened to being convinced it is a good idea. Dylan On Mon, May 18, 2015 at 02:29:58PM -0700, Mike Mason wrote: This patch allows additional tests to be excluded when resuming a test run. This is especially useful when a failing test causes a crash or reboot. Currently, that same test runs again when attempting to resume the test run, resulting in the same crash or reboot. Note that this does not change the exclude options saved at the start of the run. If a run is resumed multiple times, any additional excluded tests must be specified on the command line each time. Signed-off-by: Mike Mason michael.w.ma...@intel.com --- framework/programs/run.py | 14 +- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/framework/programs/run.py b/framework/programs/run.py index 6053074..89e4b69 100644 --- a/framework/programs/run.py +++ b/framework/programs/run.py @@ -308,6 +308,12 @@ def resume(input_): type=path.realpath, metavar=Results Path, help=Path to results folder) +parser.add_argument(-x, --exclude-tests, +default=[], +action=append, +metavar=regex, +help=Add tests to exclude + (can be used more than once)) parser.add_argument(-f, --config, dest=config_file, type=argparse.FileType(r), @@ -317,8 +323,14 @@ def resume(input_): _disable_windows_exception_messages() results = backends.load(args.results_path) + +# Additional tests to exclude from resumed run +exclude_filter = results.options['exclude_filter'] +if args.exclude_tests: +exclude_filter.extend(args.exclude_tests) + opts = core.Options(concurrent=results.options['concurrent'], -exclude_filter=results.options['exclude_filter'], +exclude_filter=exclude_filter, include_filter=results.options['filter'], execute=results.options['execute'], valgrind=results.options['valgrind'], -- 2.1.0 ___ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit signature.asc Description: Digital signature ___ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit
