On 19/10/17 19:50, Dylan Baker wrote:
Quoting Martin Peres (2017-10-19 07:17:25)
On 30/09/17 23:42, Dylan Baker wrote:
Actually CC'ing him this time....
Quoting Dylan Baker (2017-09-29 20:29:34)
Quoting Arkadiusz Hiler (2017-09-26 03:27:50)
Because in Python we have `bool([]}) == False`, providing empty test
list resulted in hitting the same code path as not providing it at all,
meaning that we run everything.
Let's just exit early with an appropriate message instead.
This will get rid of the rather surprising behavior and will help making
the execution less prone to automated list generation errors (which has
already bitten us) as well as human errors.
Signed-off-by: Arkadiusz Hiler <arkadiusz.hi...@intel.com>
---
framework/programs/run.py | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/framework/programs/run.py b/framework/programs/run.py
index 4524f171b..0fec264ec 100644
--- a/framework/programs/run.py
+++ b/framework/programs/run.py
@@ -327,6 +327,10 @@ def run(input_):
stripped = (t.split('#')[0].strip() for t in test_list)
forced_test_list = [t for t in stripped if t]
+ # to avoid running everything
+ if not forced_test_list:
+ raise exceptions.PiglitFatalError("Empty test list provided")
+
backend = backends.get_backend(args.backend)(
args.results_path,
junit_suffix=args.junit_suffix,
--
2.13.5
_______________________________________________
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit
Hmmm, there is a case that we do want to continue, and that's for resume, CC'ing
Martin to see if this breaks their use case.
Sorry for the delay! Thanks for caring about CI :)
So, I like the patch, but disagree on the "raise" here. Why not simply
print("Empty test list provided") and sys.exit(0)? There is nothing to
do after all, so why report that an error happened?
If you still want to return an error, then please make sure it is one I
can test against.
Thanks,
Martin
PiglitFatalError is actually caught by the main script, the error is printed
with 'Fatal Error: ' prepended and sys.exit(1) is called. So if that's what you
want than this is find as-is I think.
The problem is that execution errors, or abort-on also exit with error
code 1. So either we introduce a new one for this particular error, or
we just return 0 which means all went well.
Thinking more about it though, we should still generate results, just
with nothing inside (since we asked for nothing). This way, we don't
need to special case the fact that the test list is empty.
What do you think?
_______________________________________________
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit
