Bug 97686 - run-perf-tests must expand environment variables in user provided paths
Summary: run-perf-tests must expand environment variables in user provided paths
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Marcelo Lira
URL:
Keywords:
Depends on:
Blocks: 77037
  Show dependency treegraph
 
Reported: 2012-09-26 08:25 PDT by Marcelo Lira
Modified: 2012-09-26 12:20 PDT (History)
8 users (show)

See Also:


Attachments
Patch (4.05 KB, patch)
2012-09-26 08:32 PDT, Marcelo Lira
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Marcelo Lira 2012-09-26 08:25:41 PDT
Calling run-perf-tests using shell variables for paths values will result in errors when trying to open files using those paths.

Examples:

$ run-perf-tests --platform=qt --release --output-json-path=~/perf-results
$ run-perf-tests --platform=qt --release --output-json-path=$HOME/perf-results

'~' and '$HOME' don't get expanded, and further usages like the following will break:

codecs.open(path, 'w', 'utf-8')

Even worse in the case of the output path, because the error will happen only at the end of the long process of running the performance tests.
Comment 1 Marcelo Lira 2012-09-26 08:32:36 PDT
Created attachment 165810 [details]
Patch
Comment 2 Jesus Sanchez-Palencia 2012-09-26 11:47:52 PDT
(In reply to comment #1)
> Created an attachment (id=165810) [details]
> Patch

I can confirm the issue since I've faced it as well. (And it's quite annoying after you've been running tests for one hour or so.)
Comment 3 Eric Seidel (no email) 2012-09-26 12:15:50 PDT
Comment on attachment 165810 [details]
Patch

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

> Tools/Scripts/webkitpy/performance_tests/perftestsrunner.py:75
> +        def _expand_path(option, opt_str, value, parser):
> +            path = os.path.expandvars(os.path.expanduser(value))
> +            setattr(parser.values, option.dest, path)

Arguably this should use the Environment abstraction in webkitpy.common.  Otherwise, how do you test this?  It looks like currently you aren't... :)
Comment 4 WebKit Review Bot 2012-09-26 12:20:20 PDT
Comment on attachment 165810 [details]
Patch

Clearing flags on attachment: 165810

Committed r129683: <http://trac.webkit.org/changeset/129683>
Comment 5 WebKit Review Bot 2012-09-26 12:20:23 PDT
All reviewed patches have been landed.  Closing bug.