New issue
Advanced search Search tips

Issue 765783 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

run_benchmark script is broken when run locally

Project Member Reported by nedngu...@google.com, Sep 15 2017

Issue description

On my Mac, the run_benchmark script is broken:

$ ./tools/perf/run_benchmark --browser=system system_health.common_desktop --story-filter=facebook

Traceback (most recent call last):
  <module> at /Users/nednguyen/projects/chromium/src/tools/perf/run_benchmark:26
    sys.exit(main())
  main at /Users/nednguyen/projects/chromium/src/tools/perf/run_benchmark:22
    return benchmark_runner.main(config, [trybot_command.Trybot])
  main at /Users/nednguyen/projects/chromium/src/third_party/catapult/telemetry/telemetry/benchmark_runner.py:331
    command.AddCommandLineArgs(parser, environment)
  AddCommandLineArgs at /Users/nednguyen/projects/chromium/src/third_party/catapult/telemetry/telemetry/benchmark_runner.py:169
    matching_benchmarks += _MatchBenchmarkName(arg, environment)
  _MatchBenchmarkName at /Users/nednguyen/projects/chromium/src/third_party/catapult/telemetry/telemetry/benchmark_runner.py:263
    for benchmark_class in _Benchmarks(environment):
  Cacher at /Users/nednguyen/projects/chromium/src/third_party/catapult/telemetry/telemetry/decorators.py:35
    cacher.__cache[key] = obj(*args, **kwargs)
  _Benchmarks at /Users/nednguyen/projects/chromium/src/third_party/catapult/telemetry/telemetry/benchmark_runner.py:241
    index_by_class_name=True).values()
  DiscoverClasses at /Users/nednguyen/projects/chromium/src/third_party/catapult/common/py_utils/py_utils/discover.py:98
    modules = DiscoverModules(start_dir, top_level_dir, pattern)
  DiscoverModules at /Users/nednguyen/projects/chromium/src/third_party/catapult/common/py_utils/py_utils/discover.py:57
    module = __import__(module_name, fromlist=[True])
TypeError: Item in ``from list'' not a string

Reverting https://chromium-review.googlesource.com/c/chromium/src/+/604871 fixes the problem
 

Comment 1 by tiborg@chromium.org, Sep 15 2017

I saw something similar happening after I unnested the folders in the referenced CL. But the error went away for reasons I don't know. Maybe moving things back into subfolders fixes the problem?
Cc: dtu@chromium.org
Owner: nedngu...@google.com
Status: Started (was: Untriaged)
Ok, after some debugging, my tldr is __import__ is a buggy method, don't use it.

We have been using this method forever, though the fact that this bug only triggered now is a big redflag. I guess there is some sort of short circuting logic that make __import__ only type check fromlist argument if importing module_name matches certain condition.

Now stepping back, I believe the reason we need to add fromlist argument is to make sure that we import the module by name, not the top level package. 

"When the name variable is of the form package.module, normally, the top-level package (the name up till the first dot) is returned, not the module named by name. However, when a non-empty fromlist argument is given, the module named by name is returned.
" (https://docs.python.org/2/library/functions.html)

To fix this, I use https://docs.python.org/2/library/importlib.html#importlib.import_module which import the module directly.
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 16 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d15c79924126a8ed94ac652fcd4c760347d142d6

commit d15c79924126a8ed94ac652fcd4c760347d142d6
Author: catapult-deps-roller@chromium.org <catapult-deps-roller@chromium.org>
Date: Sat Sep 16 01:32:18 2017

Roll src/third_party/catapult/ 5c13bd84f..c49949828 (3 commits)

https://chromium.googlesource.com/external/github.com/catapult-project/catapult.git/+log/5c13bd84fb39..c49949828b72

$ git log 5c13bd84f..c49949828 --date=short --no-merges --format='%ad %ae %s'
2017-09-15 nednguyen [common] Update discover.py to use importlib for importing module instead of builtin __import__
2017-09-15 charliea Disable power monitoring on all desktop platforms
2017-09-15 bpastene devil: Fix args help text of video_recorder.py

Created with:
  roll-dep src/third_party/catapult
BUG= 765783 ,765383


Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, see:
http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium#TOC-Failures-due-to-DEPS-rolls


CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel
TBR=sullivan@chromium.org

Change-Id: I948184a41bc8572be7266d2c176c03ef49562381
Reviewed-on: https://chromium-review.googlesource.com/669684
Reviewed-by: <catapult-deps-roller@chromium.org>
Commit-Queue: <catapult-deps-roller@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502465}
[modify] https://crrev.com/d15c79924126a8ed94ac652fcd4c760347d142d6/DEPS

Status: Fixed (was: Started)
Components: Test>Telemetry
Components: -Speed>Telemetry

Sign in to add a comment