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

Issue 735303 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug

Blocking:
issue 713196



Sign in to add a comment

bad json results for angle gtests

Project Member Reported by dpranke@chromium.org, Jun 21 2017

Issue description

As seen in bug 713196, it looks like when you run a large test suite split across multiple shards, we log the file location for every test in a compiled binary, rather than just the tests we actually run. 

In addition, we list every test in the binary in the "all_tests" field, rather than just the tests we run.

And, it looks like the test location is actually probably wrong (or at least useless), regardless, i.e. the entries are of the form:

"dEQP_GLES3.Default/functional_attribute_location_bind_float": {
         "file": "e:\\b\\c\\b\\win\\src\\third_party\\angle\\src\\tests\\deqp_support\\angle_deqp_gtest.cpp",
         "line": 380
      },
      "dEQP_GLES3.Default/functional_attribute_location_bind_hole_float": {
         "file": "e:\\b\\c\\b\\win\\src\\third_party\\angle\\src\\tests\\deqp_support\\angle_deqp_gtest.cpp",
         "line": 380
      },

where every test has the same file location.

As a result, the log from a single test invocation is 15MB of JSON. It looks like we're running into trouble merging this data together across 10 shards.

Sergiy, can you please look into this ASAP and see what you can do to (a) make the file smaller and (b) get the locations right or disable this info if we can't?
 
Blocking: 713196
Cc: dpranke@chromium.org
Based on https://cs.chromium.org/chromium/src/third_party/angle/src/tests/deqp_support/angle_deqp_gtest.cpp?dr=C&l=380, I guess the location is same because tests are  instantiated via multiple nested macros. I am not sure if we can get much better than that...

Based on https://cs.chromium.org/chromium/src/base/test/launcher/test_results_tracker.cc?dr=C&l=221, it look like we are reporting all tests' locations here. I am now trying to figure out which method in TestResultsTracker we should add it to to only report tests that are run in current shard.
Project Member

Comment 3 by bugdroid1@chromium.org, Jun 21 2017

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

commit 9b7f56db14ce378798d654102f54af85f70b1dc6
Author: Sergiy Byelozyorov <sergiyb@chromium.org>
Date: Wed Jun 21 13:07:58 2017

Report locations only for tests that are being run as part of this shard

R=dpranke@chromium.org, phajdan.jr@chromium.org, tandrii@chromium.org

Bug:  735303 
Change-Id: I939010541f3d7f554f462d98e3cf44a86f67939d
Reviewed-on: https://chromium-review.googlesource.com/543117
Reviewed-by: Paweł Hajdan Jr. <phajdan.jr@chromium.org>
Commit-Queue: Sergiy Byelozyorov <sergiyb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#481187}
[modify] https://crrev.com/9b7f56db14ce378798d654102f54af85f70b1dc6/base/test/launcher/test_launcher.cc
[modify] https://crrev.com/9b7f56db14ce378798d654102f54af85f70b1dc6/base/test/launcher/test_results_tracker.cc
[modify] https://crrev.com/9b7f56db14ce378798d654102f54af85f70b1dc6/base/test/launcher/test_results_tracker.h

Status: Fixed (was: Assigned)
Thanks Sergiy. Is this change rolled out now? Should this fix issue 713196?
The change is rolled out. It may have fixed the issue in issue 713196, but I'd recommend you to double-check that there are no more failing builds because of MemoryError before you declare that as fixed.
Thanks!
Yes, big thanks. We'll monitor the bots as Dirk suggested.

Sign in to add a comment