Setting _CrtSetReportMode on bots?

[Hironori Bono (坊野 博典)]

Greetings Chromium developers,

(This e-mail proposes an idea to avoid a Windows-specific issue.
Please feel free to ignore this e-mail if you are not interested in
Windows.)

Today, I noticed unit_tests and installer_utils_unittests cause
timeout errors on a couple of Windows bots ("XP Tests (dbg)(1)" and
"WIn7 Tests (dbg)(1)"). To investigate these tests, these tests showed
an assertion-error dialog of VC++ and waited for users' input until
the bots killed the test processes. (I have filed Bug 132037
<http://crbug.com/132037> and added comments why these tests cause
assertion errors.) Even though this dialog is helpful for running a
test on a local PC, it is harder to debug tests that fail on bots.
(Its studio log does not always show the test that causes the
assertion error for this case.) To avoid this problem, in my personal
opinion, I would like to call _CrtSetReportMode(_CRT_ASSERT,
_CRTDBG_MODE_DEBUG) and prevent showing this assertion dialog (*1) on
bots. Would it be possible to give me your thoughts about this idea?

(*1) http://msdn.microsoft.com/en-us/library/1y71x448%28v=VS.100%29

Regards,

Hironori Bono
E-mail: hb...@chromium.org

[Alexei Svitkine]

This seems reasonable to me for test targets.

-Alexei

--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
   http://groups.google.com/a/chromium.org/group/chromium-dev

[Paweł Hajdan, Jr.]

On Mon, Jun 11, 2012 at 6:51 AM, Hironori Bono (坊野 博典) <hb...@chromium.org> wrote:

To investigate these tests, these tests showed
an assertion-error dialog of VC++ and waited for users' input until
the bots killed the test processes. (I have filed Bug 132037
<http://crbug.com/132037> and added comments why these tests cause
assertion errors.) Even though this dialog is helpful for running a
test on a local PC, it is harder to debug tests that fail on bots.
(Its studio log does not always show the test that causes the
assertion error for this case.)  To avoid this problem, in my personal
opinion, I would like to call _CrtSetReportMode(_CRT_ASSERT,
_CRTDBG_MODE_DEBUG) and prevent showing this assertion dialog (*1) on
bots. Would it be possible to give me your thoughts about this idea?

(*1) http://msdn.microsoft.com/en-us/library/1y71x448%28v=VS.100%29

Yay, great catch! Hanging on those windows is obviously Very Bad. +1 to any effort fixing this problem. Please also send the CL to me if possible.

Now one thing I'm worried about is this:

When _DEBUG is not defined, calls to _CrtSetReportMode are removed during preprocessing.

Are we seeing the hangs in Debug mode only? If so, that's not a problem.

[Greg Thompson]

On Mon, Jun 11, 2012 at 12:51 AM, Hironori Bono (坊野 博典) <hb...@chromium.org> wrote:

Greetings Chromium developers,

(This e-mail proposes an idea to avoid a Windows-specific issue.
Please feel free to ignore this e-mail if you are not interested in
Windows.)

Today, I noticed unit_tests and installer_utils_unittests cause
timeout errors on a couple of Windows bots ("XP Tests (dbg)(1)" and
"WIn7 Tests (dbg)(1)"). To investigate these tests, these tests showed
an assertion-error dialog of VC++ and waited for users' input until
the bots killed the test processes. (I have filed Bug 132037
<http://crbug.com/132037> and added comments why these tests cause
assertion errors.) Even though this dialog is helpful for running a
test on a local PC, it is harder to debug tests that fail on bots.
(Its studio log does not always show the test that causes the
assertion error for this case.)  To avoid this problem, in my personal
opinion, I would like to call _CrtSetReportMode(_CRT_ASSERT,
_CRTDBG_MODE_DEBUG) and prevent showing this assertion dialog (*1) on
bots. Would it be possible to give me your thoughts about this idea?

Would the errors appear in the buildbot logs if they were also directed to stderr?

_CrtSetReportMode(_CRT_ASSERT, _CRTDBG_MODE_DEBUG | _CRTDBG_MODE_FILE);

_CrtSetReportFile(_CRT_ASSERT, _CRTDBG_FILE_STDERR);

 

(*1) http://msdn.microsoft.com/en-us/library/1y71x448%28v=VS.100%29

Regards,

Hironori Bono
E-mail: hb...@chromium.org

[Hironori Bono (坊野 博典)]

Greetings,

Thank you all for your comments.
After sending my previous e-mail, I notice gtest has code that
suppresses pop-up dialogs of Windows (*1). Even though this code
suppresses system pop-ups and abort ones, it does not suppress CRT
pop-ups that caused this issue. I have sent an e-mail (*2) to Google
Test people so they can add code that calls _CrtSetReportMode and
_CrtSetReportFile.

(*1) http://code.google.com/searchframe#OAMlx_jo-ck/src/testing/gtest/src/gtest.cc&exact_package=chromium&q=seterrormode&type=cs&l=3870
(*2) https://groups.google.com/forum/?fromgroups#!topic/googletestframework/OjuwNlXy5ac

Regards,

Hironori Bono
E-mail: hb...@chromium.org

[Paweł Hajdan, Jr.]

We don't need to wait for gtest to do this. Feel free to make change in our test suite, with a TODO to remove it when it becomes part of gtest we use.

[Hironori Bono (坊野 博典)]

Greetings Pawel,

Thanks for your suggestion.
Unfortunately, to read the src/DEPS file, Chromium seems to pull all
files under "src/testing/gtest/" (including
"src/testing/gtest/src/gtest.cc") directly from
"http://googletest.googlecode.com/trunk" and I wonder if I can apply
local changes to its UnitTests::Run(). Would it be possible to give me
ideas that we can add these function calls into Google Test without
changing files under "testing/gtest"?

Regards,

Hironori Bono
E-mail: hb...@chromium.org

[Paweł Hajdan, Jr.]

Why don't you modify base/test/test_suite.cc?

[Hironori Bono (坊野 博典)]

Greetings Pawel,

Thanks for noticing the file.
I just too much focus on changing Google Test and did not notice the
file. (It can suppress assertion-failure dialogs to change this file
as well as to change Google Test itself.)

Regards,

Hironori Bono

On Fri, Jun 15, 2012 at 1:05 AM, Paweł Hajdan, Jr.