Skip to content

Commit

Permalink
Revert of Enable more benchmark_smoke_unittest coverage (Reland) (pat…
Browse files Browse the repository at this point in the history
…chset #2 id:20001 of https://codereview.chromium.org/1151013002/)

Reason for revert:
It looks like this is causing a failure on Mac10.9 Tests bot.
https://build.chromium.org/p/chromium.mac/builders/Mac10.9%20Tests/builds/2130

Original issue's description:
> Enable more benchmark_smoke_unittest coverage (Reland)
>
> Previously, benchmark_smoke_unittest has a heuristic to pick
> a benchmark for smoke testing if the benchmark uses a test
> in measurement directory.
> This patch removes that check, and instead provide a black
> list of benchmarks module that we exclude the
> benchmark_smoke_unittest to avoid high CQ time.
>
> This helped increases benchmark smoke coverage from 14 benchmarks -> 33 benchmarks
> (one benchmark per benchmark module).
>
> On my local linux machine, this increases the cycle time of
> benchmark_smoke_unittest from 20s -> 1m7s.
>
> To review this, the first patch is from issue 1144193002 at patchset 100001 (http://crrev.com/1144193002#ps100001)
> Second patch adds indexeddb_perf to the black list of benchmarks to be smoke tested.
>
> BUG=490130
> CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:linux_perf_bisect;tryserver.chromium.perf:mac_perf_bisect;tryserver.chromium.perf:win_perf_bisect;tryserver.chromium.perf:android_nexus5_perf_bisect
>
> Committed: https://crrev.com/2a13f1055fbba6ccb341b6fe3cab59647c876153
> Cr-Commit-Position: refs/heads/master@{#331000}

TBR=sullivan@chromium.org,dtu@chromium.org,skyostil@chromium.org,nednguyen@google.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=490130

Review URL: https://codereview.chromium.org/1152113002

Cr-Commit-Position: refs/heads/master@{#331043}
  • Loading branch information
holte authored and Commit bot committed May 22, 2015
1 parent 01b1973 commit a83337c
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 29 deletions.
39 changes: 11 additions & 28 deletions tools/perf/benchmarks/benchmark_smoke_unittest.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,22 +10,14 @@
"""

import os
import sys
import time
import unittest

from telemetry import benchmark as benchmark_module
from telemetry.core import discover
from telemetry.page import page_test
from telemetry.unittest_util import options_for_unittests
from telemetry.unittest_util import progress_reporter

from benchmarks import dom_perf
from benchmarks import indexeddb_perf
from benchmarks import rasterize_and_record_micro
from benchmarks import spaceport
from benchmarks import speedometer
from benchmarks import jetstream


def SmokeTestGenerator(benchmark):
# NOTE TO SHERIFFS: DO NOT DISABLE THIS TEST.
Expand Down Expand Up @@ -64,43 +56,34 @@ def CreatePageSet(self, options):
benchmark.ProcessCommandLineArgs(None, options)
benchmark_module.ProcessCommandLineArgs(None, options)

current = time.time()
try:
self.assertEqual(0, SinglePageBenchmark().Run(options),
msg='Failed: %s' % benchmark)
finally:
print 'Benchmark %s run takes %i seconds' % (
benchmark.Name(), time.time() - current)
self.assertEqual(0, SinglePageBenchmark().Run(options),
msg='Failed: %s' % benchmark)

return BenchmarkSmokeTest


# The list of benchmark modules to be excluded from our smoke tests.
_BLACK_LIST_TEST_MODULES = {
dom_perf, # Always fails on cq bot.
indexeddb_perf, # Always fails on Win7 & Android Tests builder.
rasterize_and_record_micro, # Always fails on cq bot.
spaceport, # Takes 451 seconds.
speedometer, # Takes 101 seconds.
jetstream, # Take 206 seconds.
}


def load_tests(loader, standard_tests, pattern):
del loader, standard_tests, pattern # unused
suite = progress_reporter.TestSuite()

benchmarks_dir = os.path.dirname(__file__)
top_level_dir = os.path.dirname(benchmarks_dir)
measurements_dir = os.path.join(top_level_dir, 'measurements')

all_measurements = discover.DiscoverClasses(
measurements_dir, top_level_dir, page_test.PageTest).values()
# Using the default of |index_by_class_name=False| means that if a module
# has multiple benchmarks, only the last one is returned.
all_benchmarks = discover.DiscoverClasses(
benchmarks_dir, top_level_dir, benchmark_module.Benchmark,
index_by_class_name=False).values()
for benchmark in all_benchmarks:
if sys.modules[benchmark.__module__] in _BLACK_LIST_TEST_MODULES:
if hasattr(benchmark, 'test') and benchmark.test not in all_measurements:
# If the benchmark does not have a measurement, then it is not composable.
# Ideally we'd like to test these as well, but the non-composable
# benchmarks are usually long-running benchmarks.
continue

# TODO(tonyg): Smoke doesn't work with session_restore yet.
if (benchmark.Name().startswith('session_restore') or
benchmark.Name().startswith('skpicture_printer')):
Expand Down
2 changes: 1 addition & 1 deletion tools/telemetry/telemetry/user_story/user_story_set.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def __init__(self, archive_data_file='', cloud_storage_bucket=None,
self._cloud_storage_bucket = cloud_storage_bucket
if base_dir:
if not os.path.isdir(base_dir):
raise ValueError('Invalid directory path of base_dir: %s' % base_dir)
raise ValueError('Must provide valid directory path for base_dir.')
self._base_dir = base_dir
else:
self._base_dir = os.path.dirname(inspect.getfile(self.__class__))
Expand Down

0 comments on commit a83337c

Please sign in to comment.