Include crypto_unittests for Valgrind, Heapcheck, and TSAN

Include crypto_unittests for Valgrind, Heapcheck, and TSAN

BUG=83183
TEST=none

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=86219

[Ryan Sleevi, 2011-05-19 05:33:40]

PTAL

[Timur Iskhodzhanov, 2011-05-19 16:51:39]

Please patch the build/all.gyp before commiting.
I'm sorry to ask you for adding a couple of lines of comments to the master.cfg
but I'm pretty sure you can formulate it better/more understandable than I can.

You may find it useful to continue the review with Lei because I'm in a
different timezone and OOO today :o)

http://codereview.chromium.org/7043031/diff/1/masters/master.chromium.memory/...
File masters/master.chromium.memory/master.cfg (right):

http://codereview.chromium.org/7043031/diff/1/masters/master.chromium.memory/...
masters/master.chromium.memory/master.cfg:233: 'tsan_crypto',
Are there any threads in crypto tests?

http://codereview.chromium.org/7043031/diff/1/masters/master.chromium.memory/...
masters/master.chromium.memory/master.cfg:360: '-target',
'chromium_builder_dbg_valgrind_mac'])
you need to add the crypto_unittests dependency to this target in build/all.gyp
first

http://codereview.chromium.org/7043031/diff/1/masters/master.chromium.memory/...
masters/master.chromium.memory/master.cfg:368: tests=['valgrind_app',
Can you please add a CAPS comment before the tests=[ line describing that one
should add the tests to the build target before adding the tests runs?

http://codereview.chromium.org/7043031/diff/1/masters/master.chromium.memory/...
masters/master.chromium.memory/master.cfg:370: 'valgrind_crypto',
How much time do they take under Valgrind on Mac?

http://codereview.chromium.org/7043031/diff/1/masters/master.chromium.memory/...
masters/master.chromium.memory/master.cfg:389: tests=['tsan_app',
ditto: comment

http://codereview.chromium.org/7043031/diff/1/masters/master.chromium.memory/...
masters/master.chromium.memory/master.cfg:526:
project='all.sln;chromium_builder_dbg_tsan_win',
ditto: you need to add crypto_unittests dependency to this target as well.

http://codereview.chromium.org/7043031/diff/1/masters/master.chromium.memory/...
File masters/master.chromium.memory/master_gatekeeper_cfg.py (right):

http://codereview.chromium.org/7043031/diff/1/masters/master.chromium.memory/...
masters/master.chromium.memory/master_gatekeeper_cfg.py:25: 'memory test:
crypto',
Be prepared to clean up the failures after restarting the bots :)

[Timur Iskhodzhanov, 2011-05-19 16:54:49]

NB: be prepared to copy some excluded test names from
tools/valgrind/gtest_exclude/base_unittests.*.txt
to
tools/valgrind/gtest_exclude/crypto_unittests.*.txt

Also, if you can wait for the master restart to happen on weekends I can help
you with restarting and doing the initial waterfall cleanup (I'm pretty sure
TSan/Win bot will be unhappy)

[Ryan Sleevi, 2011-05-20 01:35:33]

leiz: Would you mind reviewing?

One thing not addressed are the comments, as I'm not sure what needs to be said.
More importantly, it seems like more work than mentioned by Timur is involved
with adding new tests.

While not too familiar with this area, it seems that build/all.gyp needs to be
touched first, followed by chrome_tests.py in src/tools/heapcheck and
src/tools/valgrind , since they're both called by
build/scripts/master/factory/chromium_commands.py to actually run tests.

Are there other steps that need to be followed? Can you think of any good text
that I should add to document the pre-requisites? Is it OK as-is, without
addressing the comments bit?

http://codereview.chromium.org/7043031/diff/1/masters/master.chromium.memory/...
File masters/master.chromium.memory/master.cfg (right):

http://codereview.chromium.org/7043031/diff/1/masters/master.chromium.memory/...
masters/master.chromium.memory/master.cfg:233: 'tsan_crypto',
On 2011/05/19 16:51:39, Timur Iskhodzhanov wrote:
> Are there any threads in crypto tests?

Not directly, but possibly in the underlying libraries.

I added it to tsan given that all of the tests were previously covered by
tsan_base, and the goal was to restore equivalent coverage. I'm not sure if tsan
is necessary though.

http://codereview.chromium.org/7043031/diff/1/masters/master.chromium.memory/...
masters/master.chromium.memory/master.cfg:360: '-target',
'chromium_builder_dbg_valgrind_mac'])
On 2011/05/19 16:51:39, Timur Iskhodzhanov wrote:
> you need to add the crypto_unittests dependency to this target in
build/all.gyp
> first

Good catch. Thankfully, it is already there.

http://codereview.chromium.org/7043031/diff/1/masters/master.chromium.memory/...
masters/master.chromium.memory/master.cfg:370: 'valgrind_crypto',
On 2011/05/19 16:51:39, Timur Iskhodzhanov wrote:
> How much time do they take under Valgrind on Mac?

I'm not sure. Previously these tests were part of valgrind_base, which is why I
placed everything to go with the base counterpart. However, my understanding is
that tests are listed "quickest to slowest", so perhaps this isn't the right
place?

http://codereview.chromium.org/7043031/diff/1/masters/master.chromium.memory/...
masters/master.chromium.memory/master.cfg:526:
project='all.sln;chromium_builder_dbg_tsan_win',
On 2011/05/19 16:51:39, Timur Iskhodzhanov wrote:
> ditto: you need to add crypto_unittests dependency to this target as well.

Thanks. Similar with Mac, it is also already in place.

[Lei Zhang, 2011-05-20 01:56:13]

I think you just need to add the comments timurrrr asked for. Have you tried
running make start to make sure it actually starts?

[Ryan Sleevi, 2011-05-20 02:01:25]

On 2011/05/20 01:56:13, Lei Zhang wrote:
> I think you just need to add the comments timurrrr asked for. Have you tried
> running make start to make sure it actually starts?

Ok, I'll add the comments. No, I haven't actually tested any of these changes -
this is an entirely new area for me. maruel@ mentioned that commit-bot would run
a series of PRESUBMIT checks, but are there other tests I need to be run to be
sure?

Note, my primary dev box is Windows, so I'm guessing I may need to spin up
another Linux VM or switch over to Mac to test?

[Ryan Sleevi, 2011-05-20 02:11:05]

leiz: PTAL at comment updates.

[Timur Iskhodzhanov, 2011-05-20 08:39:11]

Thank you very much for the comments.
I didn't ask you for the "add to chrome_tests.py" comments but it's good to have
them as well :)

LGTM
after replacing a couple of words (see my comments)
I can restart the master for you on Saturday

http://codereview.chromium.org/7043031/diff/1/masters/master.chromium.memory/...
File masters/master.chromium.memory/master.cfg (right):

http://codereview.chromium.org/7043031/diff/1/masters/master.chromium.memory/...
masters/master.chromium.memory/master.cfg:370: 'valgrind_crypto',
Looks like the Mac/Valgrind order is closer to alphabetic or just random :)
OK, you may leave the order this way.

On 2011/05/20 01:35:33, Ryan Sleevi wrote:
> On 2011/05/19 16:51:39, Timur Iskhodzhanov wrote:
> > How much time do they take under Valgrind on Mac?
> 
> I'm not sure. Previously these tests were part of valgrind_base, which is why
I
> placed everything to go with the base counterpart. However, my understanding
is
> that tests are listed "quickest to slowest", so perhaps this isn't the right
> place?

http://codereview.chromium.org/7043031/diff/8001/masters/master.chromium.memo...
masters/master.chromium.memory/master.cfg:232: 'tsan_base',
s/sans/without ?

http://codereview.chromium.org/7043031/diff/8001/masters/master.chromium.memo...
masters/master.chromium.memory/master.cfg:372: 'valgrind_ipc',
s/appropriate/chromium_builder_dbg_valgrind_mac/

http://codereview.chromium.org/7043031/diff/8001/masters/master.chromium.memo...
masters/master.chromium.memory/master.cfg:396: 'tsan_printing',
s/appropriate/chromium_builder_dbg_valgrind_mac/

http://codereview.chromium.org/7043031/diff/8001/masters/master.chromium.memo...
masters/master.chromium.memory/master.cfg:525: ],
s/appropriate/chromium_builder_dbg_tsan_win/

[Ryan Sleevi, 2011-05-20 23:56:26]

Thanks, wording updated.

I'll hold off landing this until http://codereview.chromium.org/7039057/ has
been LGTM'd, since that seems to need to be landed first (chrome_tests.py
changes)

[commit-bot: I haz the power, 2011-05-21 16:36:09]

Can't apply patch for file masters/master.chromium.memory/master.cfg.
patching file masters/master.chromium.memory/master.cfg
Hunk #1 FAILED at 184.
Hunk #2 succeeded at 260 (offset 33 lines).
Hunk #3 succeeded at 307 (offset 33 lines).
Hunk #4 succeeded at 338 (offset 33 lines).
Hunk #5 succeeded at 356 (offset 33 lines).
Hunk #6 succeeded at 401 (offset 33 lines).
Hunk #7 succeeded at 426 (offset 33 lines).
Hunk #8 FAILED at 472.
Hunk #9 succeeded at 580 (offset 59 lines).
Hunk #10 succeeded at 592 (offset 59 lines).
2 out of 10 hunks FAILED -- saving rejects to file
masters/master.chromium.memory/master.cfg.rej

[commit-bot: I haz the power, 2011-05-21 16:42:41]

Change committed as 86219