bad json results for angle gtests |
|||
Issue descriptionAs 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?
,
Jun 21 2017
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.
,
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
,
Jun 21 2017
,
Jun 21 2017
Thanks Sergiy. Is this change rolled out now? Should this fix issue 713196?
,
Jun 21 2017
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.
,
Jun 21 2017
Thanks!
,
Jun 21 2017
Yes, big thanks. We'll monitor the bots as Dirk suggested. |
|||
►
Sign in to add a comment |
|||
Comment 1 by dpranke@chromium.org
, Jun 21 2017