Bug 145583

Summary: add errors to run-benchmark exception handling.
Product: WebKit Reporter: Stephanie Lewis <slewis>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: dewei_zhu, rniwa, slewis
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 144842    
Attachments:
Description Flags
patch rniwa: review+

Description Stephanie Lewis 2015-06-02 17:52:59 PDT
Created attachment 254128 [details]
patch

add errors to run-benchmark exception handling.
Comment 1 Ryosuke Niwa 2015-06-02 18:13:19 PDT
Comment on attachment 254128 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=254128&action=review

> Tools/Scripts/run-benchmark:42
> +    except Exception as e:

Can we spell out either exception or error instead of e?

> Tools/Scripts/webkitpy/benchmark_runner/benchmark_builder/generic_benchmark_builder.py:36
> +        except Exception as e:
>              self.clean()
> -            raise
> +            raise e

Apparently you don't need this in Python to forward the exception object.

> Tools/Scripts/webkitpy/benchmark_runner/benchmark_builder/generic_benchmark_builder.py:45
> -            raise Exception('Cannot create the benchmark', errorCode)
> +            raise Exception('Cannot create the benchmark - Error: %s' % errorCode, errorCode)

Why are we also passing errorCode as the second argument??

> Tools/Scripts/webkitpy/benchmark_runner/benchmark_runner.py:42
> +        except IOError as e:

Ditto about spelling out error or exception.

> Tools/Scripts/webkitpy/benchmark_runner/benchmark_runner.py:84
> +                result = json.loads(result)
>                  results.append(result)

We can probably just both of these in one line:
results.append(json.loads(result))

> Tools/Scripts/webkitpy/benchmark_runner/benchmark_runner.py:85
> +            except Exception as e:

Ditto about spelling out e.

> Tools/Scripts/webkitpy/benchmark_runner/generic_factory.py:23
> -            except KeyError:
> -                raise
> +            except KeyError as e:
> +                raise e

Ditto about not needing this.

> Tools/Scripts/webkitpy/benchmark_runner/generic_factory.py:31
> +        except Exception as e:
> +            raise e

Ditto.

> Tools/Scripts/webkitpy/benchmark_runner/utils.py:38
> +    except Exception as e:
> +        raise Exception("Invalid json format or empty json was found in %s - Error %s" % (filePath, e))

Spellout exception or error.

> Tools/Scripts/webkitpy/benchmark_runner/utils.py:44
> +    except Exception as e:

Ditto.
Comment 2 Stephanie Lewis 2015-06-05 02:53:54 PDT
Committed http://trac.webkit.org/changeset/185243