catapult-project / catapult

Deprecated Catapult GitHub. Please instead use http://crbug.com "Speed>Benchmarks" component for bugs and https://chromium.googlesource.com/catapult for downloading and editing source code..
https://chromium.googlesource.com/catapult
BSD 3-Clause "New" or "Revised" License
1.91k stars 563 forks source link

[📍] Plumb test errors through to the top level #4467

Closed dave-2 closed 6 years ago

dave-2 commented 6 years ago

https://bugs.chromium.org/p/chromium/issues/detail?id=843625#c5 https://pinpoint-dot-chromeperf.appspot.com/job/11605298240000

Right now, if all the Attempts fail, the bug says: All of the attempts failed. See the individual attempts for details on each error.

If you look at the individual Attempt, it says: SwarmingTestError: The swarming task 3d82b7c76fe5ae10 failed. The test exited with code 1.

If you dig into the Swarming task, it says:

Traceback (most recent call last):
  File "../../testing/scripts/run_performance_tests.py", line 271, in <module>
    sys.exit(main())
  File "../../testing/scripts/run_performance_tests.py", line 221, in main
    benchmarks = args.benchmark_names.split(',')
AttributeError: 'Namespace' object has no attribute 'benchmark_names'

Ideally, the test runner should parse the exception out of the test and say something more like:

SwarmingTestError: The test failed. The test's error message was:
AttributeError: 'Namespace' object has no attribute 'benchmark_names'

Then we can plumb that error message to the top level, so the bug says:

All of the runs failed. Here is an error from one of the runs:
SwarmingTestError: The test failed. The test's error message was:
AttributeError: 'Namespace' object has no attribute 'benchmark_names'

This will make it easier to triage, since you don't have to click through all these layers to see if it's an error you already know about. The rewording also makes it clearer to non-expert users that the tests failed.

@anniesullie @perezju @simonhatch

anniesullie commented 6 years ago

+1 this would be awesome!

perezju commented 6 years ago

Agree! This would be great! For the top level error message, wouldn't it be better if we pick the most common error message from all runs? E.g.:

All of the runs failed. The most common error (9/10 runs) was:
SwarmingTestError: The test failed. The test's error message was:
AttributeError: 'Namespace' object has no attribute 'benchmark_names'

I know this might not always work due to non-determinism in some error messages; but even then this would just naturally fall back to the original approach you had suggested.

simonhatch commented 6 years ago

+1 great idea, would also love to see something like Juan's suggestion, or barring that maybe an easy way to just show the error right then and there instead of digging it out of each run.