New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 767124 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Feature

Blocked on:
issue 533481



Sign in to add a comment

Alerting is turned on for unoptimized tests

Project Member Reported by crouleau@chromium.org, Sep 20 2017

Issue description

See crbug/766889. This causes us to send alerts out for no reason. There are two possible solutions to this:

1. Change the alerting config on chromeperf.appspot.com for Chromium AV Perf to no longer include unoptimized media_perftests.

2. Turn off those tests for the continuous runs. I'm not sure exactly how to do this, but it seems like we could filter them out before they run so that we don't waste bot time running them. However, I think they are pretty quick anyway, so it's probably not worth it.

I plan on doing solution 1. 
 
Instead of going about this in this way, we are thinking that the unoptimized tests should simply print the data in a format that the gtest perftests doesn't recognize. Apparently that is much simpler than 2. 

We don't want to do 1 because then if any additions are made to media_perftests then they will not be alerted on.
Looks like there might be a way to filter which tests to run in the continuous runs. That could be the most preferred method. I don't think printing the data in a different format is a good idea because 

1. we're still taking up bot time running tests that don't produce any useful output.
2. if we print the data in a less standard format, then it will be harder to use standard tools to analyze it in the future.

The way to filter is with https://chromium-review.googlesource.com/c/chromium/src/+/754133 We can put the name of the test to exclude in a file like
-test_name
and include the path to that file in the --isolated-script-test-filter-file argument. That argument gets remapped to the gtest argument --test-launcher-filter-file. The documentation for that argument is
" Like --gtest_filter, but read the test filter from PATH.
    One pattern per line; lines starting with '-' are exclusions.
    See also //testing/buildbot/filters/README.md file.
"
Blockedon: 533481

Comment 4 by kbr@chromium.org, Nov 16 2017

Redoing the work from  Issue 533481  to pass the filtered tests as a command line flag rather than via a file because of difficulties passing a newly generated file to Swarming.

Thanks for the heads up. Should I hold off until the redoing of the work is done?
Project Member

Comment 6 by bugdroid1@chromium.org, Dec 20 2017

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

commit 69e3e098ad571b58f831cf8649b6903512bdcb75
Author: Caleb Rouleau <crouleau@chromium.org>
Date: Wed Dec 20 02:14:32 2017

[media_perftests] Filter out unoptimized and unaligned tests.

We don't care about the performance of these, so it's best not
to run them at all.

This change has two parts:
1. Split unoptimized tests into separate test cases so that they can
be filtered separately.
2. Add the filter for them to the gn_isolate_map so that swarming jobs
will filter them out.

Bug:  767124 
Change-Id: Ia7b9a44f202346aec051280043756ea5d1d716e7
Reviewed-on: https://chromium-review.googlesource.com/835354
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Reviewed-by: John Rummell <jrummell@chromium.org>
Commit-Queue: Caleb Rouleau <crouleau@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525228}
[modify] https://crrev.com/69e3e098ad571b58f831cf8649b6903512bdcb75/media/base/sinc_resampler.h
[modify] https://crrev.com/69e3e098ad571b58f831cf8649b6903512bdcb75/media/base/sinc_resampler_perftest.cc
[modify] https://crrev.com/69e3e098ad571b58f831cf8649b6903512bdcb75/media/base/vector_math_perftest.cc
[modify] https://crrev.com/69e3e098ad571b58f831cf8649b6903512bdcb75/testing/buildbot/gn_isolate_map.pyl

Cc: kbr@chromium.org
Status: Fixed (was: Assigned)
Thanks kbr@!

Sign in to add a comment