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

Issue 894254 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocked on:
issue 894258

Blocking:
issue 739863
issue 894251



Sign in to add a comment

Support Telemetry-based benchmarks in Findit - support Findit required cmd flags

Project Member Reported by chanli@chromium.org, Oct 10

Issue description

To support telemetry-based tests in Findit, test runners need to support the following cmd flags:
1. --isolated-script-test-filter=A::B::C
2. --isolated-script-test-repeat=30
  a. To be consistent with currently supported test types, the exec 
     order should be ABCABCABC...
3. --isolated-script-test-launcher-retry-limit=0
4. --isolated-script-test-also-run-disabled-tests
 
Context: https://docs.google.com/document/d/1XZrAecLT33sJng9JTHsWCNpcBVU_BT7weLE76nwsTXA/edit?usp=sharing
 
Blocking: 739863
Owner: crouleau@chromium.org
Reassign this to Caleb since this will be instrumental to issue 739863
Cc: nedngu...@google.com
Owner: ----
Status: Available (was: Assigned)
I still can't work on this until I am done with fixing stuff for my other team. Marking this available so anyone can work on it. I may pick it back up later.
To follow up on this, nednguyen@ and crouleau@, do you have a solid plan on who should be the owner of this change and when the change will happen?
I'm still not ready to own this. I don't know of any plan yet. I won't pick it up until I am ready to start work on it. If you want it done sooner you should talk to Ned.
Cc: -crouleau@chromium.org chanli@chromium.org johnchen@chromium.org
Owner: crouleau@chromium.org
Status: Started (was: Available)
Alright, I'm on this now :)
1. Looks like we already support --isolated-script-test-filter=A::B::C

2. We support --pageset-repeat in story_runner.py. but we don't control the exec order since filtering is done via a separate path using a regex. I think that supporting exec order would be a bit of work. I'm wondering why you need the exec order to be consistent. I asked this question at https://bugs.chromium.org/p/chromium/issues/detail?id=836413#c16

3. I don't yet see retry code for Telemetry, but I'm looking still.

note from https://bugs.chromium.org/p/chromium/issues/detail?id=894251#c16
we need to support this ordering:

tests = [test1, test2, ...]
for (int i = 0; i < repeat; ++ i) {
  failed_tests = ["run tests in parallel, and return failed tests"]
  for test in failed_tests {
    for (int j = 0; j < retry-limit; ++j) {
      passed = test.run()
      if (passed)
        break;
    }
  }
}

4. We already have --also-run-disabled-tests flag in story_runner.py
2. it turns out that FindIt team doesn't care about repeat order so long as the repeats are interleaved:
i.e. for --isolated-script-test-filter=A::B --isolated-script-test-repeat=2
they are okay with either
[A, B, A, B]
or
[B, A, B, A]

See https://bugs.chromium.org/p/chromium/issues/detail?id=836413#c22
Project Member

Comment 9 by bugdroid1@chromium.org, Nov 30

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

commit aedea6feaf083a0876b70107c22f027e1d97b082
Author: Caleb Rouleau <crouleau@chromium.org>
Date: Fri Nov 30 02:27:16 2018

startup benchmarks cleanup: remove 'pageset_repeat': 1'

This code doesn't do anything:

The pageset_repeat option is used to set the default for the flag
'--pageset-repeat'. The default is 1. If someone passes in
--pageset-repeat=3 over the commandline then that will override
this option either way.

See https://goo.gl/JZ2yZQ. I am attempting to redesign this
system a bit, and I noticed this code.

Bug: 894254
Change-Id: I80da537180384f8b4b4ed25c87f9c9c8f814b2d5
Reviewed-on: https://chromium-review.googlesource.com/c/1356131
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Commit-Queue: Ned Nguyen <nednguyen@google.com>
Cr-Commit-Position: refs/heads/master@{#612525}
[modify] https://crrev.com/aedea6feaf083a0876b70107c22f027e1d97b082/tools/perf/benchmarks/startup_mobile.py

Project Member

Comment 10 by bugdroid1@chromium.org, Nov 30

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

commit 5f53d96079abaefd2bb29f19a5981b3d429344ee
Author: Caleb Rouleau <crouleau@chromium.org>
Date: Fri Nov 30 23:30:10 2018

[media telemetry] Don't edit commandline flag defaults to exclude stories.

Remove "options = {'story_tag_filter_exclude': 'is_4k,is_50fps'}"
This isn't really a good way to do this because of the following.

if you run the code like this:
$ ./tools/perf/run_benchmark run media.mobile --browser=android-chrome
then the filter will work:
   number of stories 22

but if you run the code like this then
$ ./tools/perf/run_benchmark run media.mobile --browser=android-chrome --story-tag-filter-exclude='other_tag'
then the filter will not work:
   number of stories 29 (which means the 4k stories and 50fps stories were not excluded.)
This is because the "options" simply sets the default "--story-tag-filter-exclude"
So when a user provides a different value, the default is overriden.

My change fixes this such that you can use that flag without side effects.
It also helps by making the other parts of the system be able to know
by default what stories are in the page_set. You can see that this is
true because the the autogenerated benchmark.csv file now has fewer
tags for media.mobile because of the filtered stories.


I am attempting to redesign this
system a bit for https://goo.gl/JZ2yZQ. I noticed this code.


Also remove
tag = 'android'
since it doesn't do anything.


Bug: 894254
Change-Id: Ibf5f90e83062c785934af334c8dd6bfde89cef25
Reviewed-on: https://chromium-review.googlesource.com/c/1356320
Commit-Queue: Caleb Rouleau <crouleau@chromium.org>
Reviewed-by: Juan Antonio Navarro Pérez <perezju@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612840}
[modify] https://crrev.com/5f53d96079abaefd2bb29f19a5981b3d429344ee/tools/perf/benchmark.csv
[modify] https://crrev.com/5f53d96079abaefd2bb29f19a5981b3d429344ee/tools/perf/benchmarks/media.py
[modify] https://crrev.com/5f53d96079abaefd2bb29f19a5981b3d429344ee/tools/perf/page_sets/media_cases.py

Project Member

Comment 11 by bugdroid1@chromium.org, Dec 4

The following revision refers to this bug:
  https://chromium.googlesource.com/catapult/+/4feee5840de24533ab336d366cf632c2f5210510

commit 4feee5840de24533ab336d366cf632c2f5210510
Author: Caleb Rouleau <crouleau@chromium.org>
Date: Tue Dec 04 00:16:25 2018

[Telemetry] Deprecate --smoke-test-mode flag.

Instead, you can simply use --pageset-repeat=1.
This flag currently has no other purpose.

Note that we currently support duplicate conflicting flags:
for example
‘run_benchmark ... --pageset-repeat=2 --pageset-repeat=3’
We use the last flag, 3 in the example.

Note that https://chromium-review.googlesource.com/c/chromium/src/+/1357722
needs to be checked in before this change is checked in.

as explained in https://goo.gl/JZ2yZQ Proposal B, this change is
essential for supporting the FindIt use case.

Bug: chromium:894254
Change-Id: Ieed68e0f584cd1474738d8125f6872bfb978e1f0
Reviewed-on: https://chromium-review.googlesource.com/c/1356903
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Reviewed-by: Emily Hanley <eyaich@chromium.org>

[modify] https://crrev.com/4feee5840de24533ab336d366cf632c2f5210510/telemetry/telemetry/internal/story_runner.py
[modify] https://crrev.com/4feee5840de24533ab336d366cf632c2f5210510/telemetry/telemetry/internal/story_runner_unittest.py

Project Member

Comment 12 by bugdroid1@chromium.org, Dec 4

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

commit 5067b69c8d75e8667d089aa2cd93bc7bbf4b3999
Author: Caleb Rouleau <crouleau@chromium.org>
Date: Tue Dec 04 00:15:12 2018

[Telemetry][CI] Replace --smoke-test-mode with --pageset-repeat=1

--smoke-test-mode's only effect is to set pageset-repeat to 1.
This is only true for benchmark runs. For merge scripts,
that flag has different meaning and does not need to be removed.

Note that Note that we currently support duplicate conflicting flags:
for example
‘run_benchmark ... --pageset-repeat=2 --pageset-repeat=3’
We use the last flag, 3 in the example.

--smoke-test-mode is getting deprecated in
https://chromium-review.googlesource.com/c/catapult/+/1356903


Bug: 894254
Change-Id: I1b0e423167018f6010f66255838ad8a33d6acd76
Reviewed-on: https://chromium-review.googlesource.com/c/1357722
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Reviewed-by: Emily Hanley <eyaich@chromium.org>
Reviewed-by: John Budorick <jbudorick@chromium.org>
Commit-Queue: Caleb Rouleau <crouleau@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613342}
[modify] https://crrev.com/5067b69c8d75e8667d089aa2cd93bc7bbf4b3999/testing/buildbot/chromium.android.fyi.json
[modify] https://crrev.com/5067b69c8d75e8667d089aa2cd93bc7bbf4b3999/testing/buildbot/chromium.linux.json
[modify] https://crrev.com/5067b69c8d75e8667d089aa2cd93bc7bbf4b3999/testing/buildbot/chromium.mac.json
[modify] https://crrev.com/5067b69c8d75e8667d089aa2cd93bc7bbf4b3999/testing/buildbot/chromium.win.json
[modify] https://crrev.com/5067b69c8d75e8667d089aa2cd93bc7bbf4b3999/testing/buildbot/test_suites.pyl
[modify] https://crrev.com/5067b69c8d75e8667d089aa2cd93bc7bbf4b3999/tools/perf/benchmarks/benchmark_smoke_unittest.py

Project Member

Comment 13 by bugdroid1@chromium.org, Dec 4

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

commit 99980056300b7b80a399554db03bd3fedfba34c7
Author: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Date: Tue Dec 04 02:09:26 2018

Roll src/third_party/catapult 4f870714be35..4feee5840de2 (1 commits)

https://chromium.googlesource.com/catapult.git/+log/4f870714be35..4feee5840de2


git log 4f870714be35..4feee5840de2 --date=short --no-merges --format='%ad %ae %s'
2018-12-04 crouleau@chromium.org [Telemetry] Deprecate --smoke-test-mode flag.


Created with:
  gclient setdep -r src/third_party/catapult@4feee5840de2

The AutoRoll server is located here: https://autoroll.skia.org/r/catapult-autoroll

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

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel

BUG=chromium:894254
TBR=sullivan@chromium.org

Change-Id: I60b05d06fbd591a7c80bae9787e415abeea4de97
Reviewed-on: https://chromium-review.googlesource.com/c/1359114
Reviewed-by: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#613392}
[modify] https://crrev.com/99980056300b7b80a399554db03bd3fedfba34c7/DEPS

Hi Caleb, any update on this bug? Is the test runner supporting all the cmd flags that Findit needs now? 
Still working on it. Other more pressing problems have been stealing my focus, but I hope to get back to this soon.
Thank you for the updates!
Blockedon: 894258
Blocking: 916762
Labels: -Pri-3 Pri-1
This is critical - it's preventing the Telemetry-based GPU tests from showing up in FindIt, so we are completely blind to regressions like  Issue 916544 . For this reason I'm upgrading this to P1.

These tests were upgraded to support these command line flags in  Issue 894258 .

Note: if FindIt can whitelist the GPU based tests per https://bugs.chromium.org/p/chromium/issues/detail?id=916762#c6 then this would no longer block  Issue 916762  and it can be downgraded again -- though I strongly recommend fixing this once and for all.

I should have some cycles next week to test out the swarming tasks using new flags on GPU based tests. If that works properly, I can start preparing Findit to support GPU based tests. I strongly agree that it'll be better if we can have all test types ready though.
Agreed. This is important.
3. I spoke with chanli and it sound like Findit is going to always set --isolated-script-test-launcher-retry-limit to 0. So instead of implementing the retries, I will just check that flag and if it is anything other than 0 I will hard fail.
Project Member

Comment 22 by bugdroid1@chromium.org, Dec 21

The following revision refers to this bug:
  https://chromium.googlesource.com/catapult/+/d6891dfeca7882a9d5398a730e4150091e10a4dd

commit d6891dfeca7882a9d5398a730e4150091e10a4dd
Author: Caleb Rouleau <crouleau@chromium.org>
Date: Fri Dec 21 11:11:34 2018

[Telemetry] Remove deprecated --smoke-test-mode flag

Bug: chromium:894254
Change-Id: I1800d393831621482ddfc4d7d25d1fb6f2897cea
Reviewed-on: https://chromium-review.googlesource.com/c/1387964
Commit-Queue: Juan Antonio Navarro Pérez <perezju@chromium.org>
Auto-Submit: Caleb Rouleau <crouleau@chromium.org>
Reviewed-by: Juan Antonio Navarro Pérez <perezju@chromium.org>

[modify] https://crrev.com/d6891dfeca7882a9d5398a730e4150091e10a4dd/telemetry/telemetry/internal/story_runner.py

Project Member

Comment 23 by bugdroid1@chromium.org, Dec 21

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

commit 6de01bf77212792a1307576105fd75c0ae652e2f
Author: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Date: Fri Dec 21 16:30:09 2018

Roll src/third_party/catapult 2f6c18c2d5aa..4ee958e1f521 (5 commits)

https://chromium.googlesource.com/catapult.git/+log/2f6c18c2d5aa..4ee958e1f521


git log 2f6c18c2d5aa..4ee958e1f521 --date=short --no-merges --format='%ad %ae %s'
2018-12-21 perezju@chromium.org Revert "Adds ability to pass tags to Typ from browser test suites"
2018-12-21 crouleau@chromium.org [Telemetry] Remove deprecated --smoke-test-mode flag
2018-12-21 perezju@chromium.org [Telemetry] Remove LegacyPageTest.TabForPage
2018-12-21 lwsong@google.com Fixes for Metrics Visualization
2018-12-21 rmhasan@google.com Adds ability to pass tags to Typ from browser test suites


Created with:
  gclient setdep -r src/third_party/catapult@4ee958e1f521

The AutoRoll server is located here: https://autoroll.skia.org/r/catapult-autoroll

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

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel

BUG= chromium:835690 ,chromium:894254, chromium:851948 , chromium:835690 
TBR=sullivan@chromium.org

Change-Id: Ic0ce5e1219d7c4582c1ac05862f18feb95b401cf
Reviewed-on: https://chromium-review.googlesource.com/c/1388379
Reviewed-by: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#618545}
[modify] https://crrev.com/6de01bf77212792a1307576105fd75c0ae652e2f/DEPS

Blocking: -916762
Project Member

Comment 25 by bugdroid1@chromium.org, Dec 21

The following revision refers to this bug:
  https://chromium.googlesource.com/catapult/+/442e7b2731071e114fd7382afdcd6d5dc1248795

commit 442e7b2731071e114fd7382afdcd6d5dc1248795
Author: Caleb Rouleau <crouleau@chromium.org>
Date: Fri Dec 21 19:40:43 2018

[Telemetry] Remove deprecated --output-file flag.

This is just a cleanup as I work on issue 894254.

This was deprecated as part of https://codereview.chromium.org/616063004/

Bug: chromium:894254
Change-Id: I32c50749d5b785d104c587202d3193c89af4ca62
Reviewed-on: https://chromium-review.googlesource.com/c/1387681
Commit-Queue: Caleb Rouleau <crouleau@chromium.org>
Auto-Submit: Caleb Rouleau <crouleau@chromium.org>
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Reviewed-by: Juan Antonio Navarro Pérez <perezju@chromium.org>

[modify] https://crrev.com/442e7b2731071e114fd7382afdcd6d5dc1248795/telemetry/telemetry/internal/results/results_options.py
[modify] https://crrev.com/442e7b2731071e114fd7382afdcd6d5dc1248795/telemetry/telemetry/internal/story_runner.py
[modify] https://crrev.com/442e7b2731071e114fd7382afdcd6d5dc1248795/telemetry/telemetry/internal/results/json_3_output_formatter_unittest.py

Per  Issue 916762  it turns out the lack of data for the GPU tests in FindIt seems to have been caused by disabling of the "retry with patch" step in the Chromium recipe. So while I think this is still important, it doesn't seem to be a blocker for that other bug any more.

Labels: -Pri-1 Pri-2
I think P1 is more for stuff that is broken. This is a feature request, so reducing it back to P2. I'm still working on it however, and I hope to finish it up before I'm interrupted by another actual P1.
Blockedon: 757933
Blocked on issue 757933 since testing/scripts/run_performance_tests.py currently supports both gtest perf tests and telemetry tests, which is making it painful to migrate it to using testing/scripts/common.py's BaseIsolatedScriptArgsAdapter.
Project Member

Comment 29 by bugdroid1@chromium.org, Dec 22

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

commit 6ba94872cc719d57a8af4612a22969d54ae6ba19
Author: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Date: Sat Dec 22 00:03:21 2018

Roll src/third_party/catapult f396a42a943f..442e7b273107 (1 commits)

https://chromium.googlesource.com/catapult.git/+log/f396a42a943f..442e7b273107


git log f396a42a943f..442e7b273107 --date=short --no-merges --format='%ad %ae %s'
2018-12-21 crouleau@chromium.org [Telemetry] Remove deprecated --output-file flag.


Created with:
  gclient setdep -r src/third_party/catapult@442e7b273107

The AutoRoll server is located here: https://autoroll.skia.org/r/catapult-autoroll

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

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel

BUG=chromium:894254
TBR=sullivan@chromium.org

Change-Id: I27b5960a43bdee3247ed1379c14629d20b6064c7
Reviewed-on: https://chromium-review.googlesource.com/c/1388996
Reviewed-by: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#618683}
[modify] https://crrev.com/6ba94872cc719d57a8af4612a22969d54ae6ba19/DEPS

📍 Job complete. See results below.
https://pinpoint-dot-chromeperf.appspot.com/job/14ca91a9940000
Project Member

Comment 32 by bugdroid1@chromium.org, Jan 11

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

commit 783044b79865a90376d34dd786d91afa82186aee
Author: Caleb Rouleau <crouleau@chromium.org>
Date: Fri Jan 11 00:57:00 2019

Support FindIt arguments in Telemetry benchmark runner.

I originally was trying to refactor the world to clean this
whole thing up, but I think this is much easier. We can refactor
later. This unblocks FindIt team now.

Anyone know any ways for me to test to ensure this change will not
break the waterfall? We don't have try jobs anymore, but I started
a pinpoint job here:
https://pinpoint-dot-chromeperf.appspot.com/job/14ca91a9940000
Not sure whether it will show anything though.

Bug: 894254
Change-Id: Ibf45a912b96904a2556b130f3bb5491ef534d935
Reviewed-on: https://chromium-review.googlesource.com/c/1404425
Reviewed-by: Chan Li <chanli@chromium.org>
Reviewed-by: Emily Hanley <eyaich@chromium.org>
Reviewed-by: Brian Sheedy <bsheedy@chromium.org>
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Commit-Queue: Caleb Rouleau <crouleau@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621838}
[modify] https://crrev.com/783044b79865a90376d34dd786d91afa82186aee/testing/scripts/run_telemetry_benchmark_as_googletest.py
[modify] https://crrev.com/783044b79865a90376d34dd786d91afa82186aee/tools/perf/scripts_smoke_unittest.py

Status: Fixed (was: Started)
This is fixed. Chan, can you please verify?
Caleb, could you give me some example builds/steps I can use to verify?
I figured you would know how to do that. I wrote an end-to-end test for the flags in my changelist, but it doesn't cover everything. I assumed that you could just modify the FindIt recipes referenced in issue 894485 and run them and see if they work. Probably if you don't know how to do that then you should ask your TL for help.

performance_test_suite from https://ci.chromium.org/p/chrome/builders/luci.chrome.ci/win-10-perf/1824 is modified to accept those arguments, but I have no use for those arguments, so they aren't used in my recipe.
I already have a testing script, all I need is an example build (like https://ci.chromium.org/p/chrome/builders/luci.chrome.ci/win-10-perf/1824 you provided) to test.

I'll post the results later.
For some reason I can not get swarming task info in this build (or any build on this builder) so that I cannot create a new task request based on it. I have asked maruel@, will update later.
Cc: st...@chromium.org
OK, the reason is that the build you gave me is on chrome project rather than chromium.

I managed to find some chromium builds I can use to test.

Here is a sample task: https://chromium-swarm.appspot.com/task?id=426d91500196c010&refresh=10&request_detail=true&show_raw=1

Which is referring task https://chromium-swarm.appspot.com/task?id=426a8d3035eb6510&refresh=10&request_detail=true&show_raw=1 from build https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Win10%20Tests%20x64%20%28dbg%29/5513.

The sample task runs up, but it runs 0 tests.

Could you take a look? 

Comment 39 by chanli@chromium.org, Jan 17 (5 days ago)

Status: Assigned (was: Fixed)
Reopen the bug for now. We can close it again after the question is cleared.

Comment 40 by crouleau@chromium.org, Jan 18 (5 days ago)

Blockedon: -757933

Comment 41 by crouleau@chromium.org, Jan 18 (5 days ago)

So it seems like --isolated-script-test-filter needs to support taking in a different format. Right now my isolated output has a file at dummy_benchmark.noisy_benchmark_1/test_results.json

https://isolateserver.appspot.com/browse?namespace=default-gzip&digest=faf628c09b4f7cc9675bac2a72f2639e47c273c5&as=dummy_benchmark.noisy_benchmark_1%5Ctest_results.json

that looks like this:

{
  "tests": {
    "dummy_benchmark.noisy_benchmark_1": {
      "dummy_page.html": {
        "actual": "FAIL",
        "artifacts": {
          "logs": [
            "artifacts/telemetry_test8nudzx"
          ]
        },
        "is_unexpected": true,
        "times": [
          24.782000064849854
        ],
        "time": 24.782000064849854,
        "expected": "PASS"
      }
    }
  },
  "interrupted": false,
  "num_failures_by_type": {
    "FAIL": 1
  },
  "version": 3,
  "seconds_since_epoch": 1547559677.796,
  "path_delimiter": "/"
}

Chan says over email:

"
1. for other test types, there's usually a file called 'output.json', it's a summary file that contains all tests and their results that run in the task. And Findit uses that file to get test info we need.
2. For other test types using the json_test_result_format, usually test name is a path along the trie, meaning "dummy_benchmark.noisy_benchmark_1/dummy_page.html" is a test name.
"

Currently my output.json is named test_results.json, so I can rename it to output.json.

Where do you need the output.json file to be? Does it need to be directly inside the isolated output, or is it okay for it to be under a folder like it is in 
https://isolateserver.appspot.com/browse?namespace=default-gzip&hash=3f4044e0000b5726b9a18ebc04209caab4a95ffc

It sounds like also I need to support --isolated-script-test-filter=dummy_benchmark.noisy_benchmark_1/dummy_page.html , whereas now I support --isolated-script-test-filter=dummy_page.html

Comment 42 by crouleau@chromium.org, Jan 18 (5 days ago)

Also, it looks like run_performance_tests.py is ignoring --isolated-script-test-output right now :( 

Comment 43 by chanli@chromium.org, Jan 18 (5 days ago)

(Copied my response in the email to the bug as well).

So the output.json file should be a merged version of all test_results.json files - it should be a big trie includes results of all test run in the task. In that sense, we'd love to have an output.json file directly inside the isolated output.

Sign in to add a comment