Jobs complete successfully on swarming but show up as failed on buildbot |
||||||
Issue descriptionhttps://chromium-swarm.appspot.com/task?id=3971c490bf0ace10&refresh=10&show_raw=1 and https://build.chromium.org/p/tryserver.chromium.win/builders/win10_chromium_x64_rel_ng/builds/13709 From https://chromium-review.googlesource.com/c/chromium/src/+/737439 I'm probably holding something wrong, but not clear what.
,
Oct 26 2017
,
Oct 26 2017
Scottmg says: """ I think this is because swarming runs require the use of the stuff in the src test launcher to get --test-launcher-summary-output to get the results back as .json, but that's not in standard gtest that crashpad tests are using.""" Is there any way forward here for these tests then?
,
Oct 26 2017
,
Oct 26 2017
To clarify, the swarming recipe, not swarming itself. https://cs.chromium.org/chromium/build/scripts/slave/recipe_modules/swarming/api.py?l=477
,
Oct 26 2017
(I'm not sure that the work in crbug.com/743139 has much payoff. :/) I guess we could do something similar to what we do with base for crashpad and build against a different main() (the test harness launcher) so that it gets --test-launcher-summary-output. That'll mostly require a variety of build file fiddling to switch entry points in and out. I don't think Crashpad uses any custom special main.ccs, rather all using test.gyp:crashpad_gtest_main, so they should be fairly easily switchable. Alternatively, we could let the swarming recipe respect the exit code and not report the richer information from the .json (which would have been a lot easier for Fuchsia bots too, though not as good in the long term). I wouldn't do the third option of having crashpad's test runner emit the required json as it's pretty adhoc and complex.
,
Oct 27 2017
I’m also not too interested in duplicating Chromium’s test launcher stuff in mini_chromium. If running the tests on Chromium bots for swarming requires a nonstandard gtest/gmock main() launcher, then we should probably just use Chromium’s when running Crashpad tests in Chromium. Crashpad doesn’t use the standard gtest/gmock-provided main(), it does need to do a couple of things to the test environment before invoking tests: https://chromium.googlesource.com/crashpad/crashpad/+/5e9ed4cb9f69ffc60e47d7e5d8312f6417dcb323/test/gtest_main.cc https://chromium.googlesource.com/crashpad/crashpad/+/5e9ed4cb9f69ffc60e47d7e5d8312f6417dcb323/test/gmock_main.cc There are two custom things we do beyond jumping into the tests: we save off the argc and argv passed to main() (this is for our test that checks that we’re able to read argv from out of process), and we register the new dynamic test disabling stuff (https://chromium.googlesource.com/crashpad/crashpad/+/c49da9caef302ad03f7161c58edf34825fd0171e/test/gtest_disabled.h) with gtest. Looking around at things that call base::LaunchUnitTests, it seems that there’s precedent for modules that need it to have their own main() do set-up. We should be able to do that for Crashpad’s tests too. Using https://codesearch.chromium.org/search/?q=testing::AddGlobalTestEnvironment+base::LaunchUnitTests as a guide, it appears that there wouldn’t be any trouble with using a main() that does Crashpad-specific setup and then lets Chromium’s test launcher take over. This would basically be the same as Crashpad’s current test main()s, except it’d use base::LaunchUnitTests() instead of gtest’s RUN_ALL_TESTS(). In fact, we could even just do this in Crashpad’s existing test main()s with an #ifdef that detected that it was being built in Chromium.
,
Oct 27 2017
is there a define to detect that already?
,
Oct 27 2017
"that" being "crashpad is being built as part of chromium", that is.
,
Oct 27 2017
We used to have one when we shared a build system, but we tore it out when Chromium deleted its .gyp files. My suggestion would be to define CRASHPAD_IN_CHROMIUM in the Crashpad BUILD.gn files. Really, we don’t want the shippy parts of Chromium Crashpad to be different from non-Chromium Crashpad. The switchiness was mostly just to deal with a few paths being different when Crashpad’s in Chrome’s tree. An adaptation to fitting into Chromium’s test infrastructure is just fine, though.
,
Oct 27 2017
Or just pick up defined(CHROMIUM_BUILD) || defined(GOOGLE_CHROME_BUILD).
,
Oct 27 2017
Probably better to do this here than on the review.
I did the "if in chromium, use TestLauncher" thing Mark suggested on the swarming bug, see latest patch set (even though it seems a bit unfortunate that swarming doesn't just work with gtest). Things link, but at least one test fails under TestLauncher that was happy before:
C:\src\chrome\src>out\gn\crashpad_util_test.exe --gtest_filter=ExceptionHandlerServerTest.MultipleConnections --single-process-tests
Note: Google Test filter = ExceptionHandlerServerTest.MultipleConnections
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from ExceptionHandlerServerTest
[ RUN ] ExceptionHandlerServerTest.MultipleConnections
IMPORTANT DEBUGGING NOTE: batches of tests are run inside their
own process. For debugging a test inside a debugger, use the
--gtest_filter=<your_test_name> flag along with
--single-process-tests.
Using sharding settings from environment. This is shard 0/1
Using 1 parallel jobs.
Note: Google Test filter = ExceptionHandlerServerTest.MultipleConnections
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from ExceptionHandlerServerTest
[ RUN ] ExceptionHandlerServerTest.MultipleConnections
[4036:8948:1027/122737.451:767767765:ERROR:file_io.cc(134)] WriteFile: The handle is invalid. (0x6)
[4036:8948:1027/122737.458:767767765:FATAL:file_io.cc(146)] Check failed: LoggingWriteFile(file, buffer, size).
Backtrace:
base::debug::StackTrace::StackTrace [0x008BBDE0+32]
base::debug::StackTrace::StackTrace [0x008BB76D+13]
logging::LogMessage::~LogMessage [0x008B02FE+78]
crashpad::CheckedWriteFile [0x008A1BED+81]
crashpad::test::WinChildProcess::WinChildProcess [0x00901CC0+456]
std::unique_ptr<crashpad::test::WinChildProcess::Handles,std::default_delete<crashpad::test::WinChildProcess::Handles> >::~unique_pt
r<crashpad::test::WinChildProcess::Handles,std::default_delete<crashpad::test::WinChildProcess::Handles> > [0x0087E579+97]
base::trace_event::HeapProfilerEventFilter::`scalar deleting destructor' [0x0087E22A+1418]
testing::Test::Run [0x008992E5+193]
testing::TestInfo::Run [0x00899967+213]
testing::TestCase::Run [0x00899D32+246]
testing::internal::UnitTestImpl::RunAllTests [0x0089DC8E+606]
testing::UnitTest::Run [0x0089D954+178]
base::TestSuite::Run [0x0090B104+100]
std::unique_ptr<logging::ScopedLogAssertHandler,std::default_delete<logging::ScopedLogAssertHandler> >::reset [0x0090BC24+294]
base::LaunchUnitTestsSerially [0x0090C031+105]
main [0x0090AF51+121]
__scrt_common_main_seh [0x0095609A+248] (f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:283)
BaseThreadInitThunk [0x7686336A+18]
RtlInitializeExceptionChain [0x773298F2+99]
RtlInitializeExceptionChain [0x773298C5+54]
[1/1] ExceptionHandlerServerTest.MultipleConnections (CRASHED)
1 test crashed:
ExceptionHandlerServerTest.MultipleConnections (../../third_party/crashpad/crashpad/util/win/exception_handler_server_test.cc:183)
Tests took 0 seconds.
https://cs.chromium.org/chromium/src/third_party/crashpad/crashpad/test/win/win_child_process.cc?type=cs&sq=package:chromium&l=159
,
Oct 27 2017
Also
[ RUN ] ScopedProcessSuspend.ScopedProcessSuspend
[14080:9412:1027/123536.655:768246954:ERROR:file_io.cc(134)] WriteFile: The handle is invalid. (0x6)
[14080:9412:1027/123536.657:768246954:FATAL:file_io.cc(146)] Check failed: LoggingWriteFile(file, buffer, size).
Backtrace:
base::debug::StackTrace::StackTrace [0x0031BDE0+32]
base::debug::StackTrace::StackTrace [0x0031B76D+13]
logging::LogMessage::~LogMessage [0x003102FE+78]
crashpad::CheckedWriteFile [0x00301BED+81]
crashpad::test::WinChildProcess::WinChildProcess [0x00361CC0+456]
std::vector<crashpad::CheckedRange<unsigned __int64,unsigned __int64>,std::allocator<crashpad::CheckedRange<unsigned __int64,unsigne
d __int64> > >::_Tidy [0x002E9899+24615]
std::vector<crashpad::CheckedRange<unsigned __int64,unsigned __int64>,std::allocator<crashpad::CheckedRange<unsigned __int64,unsigne
d __int64> > >::_Tidy [0x002E9335+23235]
testing::Test::Run [0x002F92E5+193]
testing::TestInfo::Run [0x002F9967+213]
testing::TestCase::Run [0x002F9D32+246]
testing::internal::UnitTestImpl::RunAllTests [0x002FDC8E+606]
testing::UnitTest::Run [0x002FD954+178]
base::TestSuite::Run [0x0036B104+100]
std::unique_ptr<logging::ScopedLogAssertHandler,std::default_delete<logging::ScopedLogAssertHandler> >::reset [0x0036BC24+294]
base::LaunchUnitTestsSerially [0x0036C031+105]
main [0x0036AF51+121]
__scrt_common_main_seh [0x003B609A+248] (f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:283)
BaseThreadInitThunk [0x7686336A+18]
RtlInitializeExceptionChain [0x773298F2+99]
RtlInitializeExceptionChain [0x773298C5+54]
[1/1] ScopedProcessSuspend.ScopedProcessSuspend (CRASHED)
1 test crashed:
ScopedProcessSuspend.ScopedProcessSuspend (../../third_party/crashpad/crashpad/util/win/scoped_process_suspend_test.cc:92)
,
Oct 27 2017
OK, I’ll say it again here. Ah, the WinMultiprocess (--is-multiprocess-child) tests, of which this is one, are a little weird and fragile.
,
Oct 27 2017
With those two disabled, things pass locally at least, even when not running serially.
,
Oct 27 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/00a0654929787f70b0cd81f30aa14e81c5e11b2f commit 00a0654929787f70b0cd81f30aa14e81c5e11b2f Author: Nico Weber <thakis@chromium.org> Date: Fri Oct 27 19:20:10 2017 Build and run crashpad_util_test on Windows bots. Ran python tools\mb\mb.py run out\gn crashpad_util_test to locally verify data and data_deps entries for swarming. Swarming apparently requires the use of base::TestLauncher, so use that in Crashpad-in-Chromium builds. Disable a bunch of tests that fail in that setup. Bug: 777924 , 778672 Change-Id: I4aa828040d488755582e5559d5f117e1ed8d1c86 Reviewed-on: https://chromium-review.googlesource.com/737439 Commit-Queue: Nico Weber <thakis@chromium.org> Reviewed-by: Dirk Pranke <dpranke@chromium.org> Reviewed-by: Mark Mentovai <mark@chromium.org> Cr-Commit-Position: refs/heads/master@{#512262} [modify] https://crrev.com/00a0654929787f70b0cd81f30aa14e81c5e11b2f/build/secondary/third_party/crashpad/crashpad/test/BUILD.gn [modify] https://crrev.com/00a0654929787f70b0cd81f30aa14e81c5e11b2f/build/secondary/third_party/crashpad/crashpad/util/BUILD.gn [modify] https://crrev.com/00a0654929787f70b0cd81f30aa14e81c5e11b2f/testing/buildbot/chromium.clang.json [modify] https://crrev.com/00a0654929787f70b0cd81f30aa14e81c5e11b2f/testing/buildbot/chromium.fyi.json [modify] https://crrev.com/00a0654929787f70b0cd81f30aa14e81c5e11b2f/testing/buildbot/chromium.win.json [modify] https://crrev.com/00a0654929787f70b0cd81f30aa14e81c5e11b2f/testing/buildbot/gn_isolate_map.pyl [modify] https://crrev.com/00a0654929787f70b0cd81f30aa14e81c5e11b2f/third_party/crashpad/crashpad/test/gmock_main.cc [modify] https://crrev.com/00a0654929787f70b0cd81f30aa14e81c5e11b2f/third_party/crashpad/crashpad/test/gtest_main.cc [modify] https://crrev.com/00a0654929787f70b0cd81f30aa14e81c5e11b2f/third_party/crashpad/crashpad/util/win/exception_handler_server_test.cc [modify] https://crrev.com/00a0654929787f70b0cd81f30aa14e81c5e11b2f/third_party/crashpad/crashpad/util/win/process_info_test.cc [modify] https://crrev.com/00a0654929787f70b0cd81f30aa14e81c5e11b2f/third_party/crashpad/crashpad/util/win/scoped_process_suspend_test.cc
,
Oct 27 2017
Fixed what I wanted to fix here. It feels like infra could do more work here (have an understandable error; make vanilla gtest tests work on swarming directly), but I'm happy here for now.
,
Oct 27 2017
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by thakis@chromium.org
, Oct 26 2017