Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(296)

Issue 2984303002: Split the testing class into two. (Closed)

Created:
3 years, 4 months ago by njanevsk
Modified:
3 years, 4 months ago
CC:
syzygy-changes_googlegroups.com, etienneb
Target Ref:
refs/heads/master
Project:
syzygy
Visibility:
Public.

Description

Split the testing class into two. The Asan tests are run as parametrized test with sygyasan dll and clang instrumented dll. The remaining tests are run as regular test fixture. To get this working the testing class was split into LenientInstrumentAppIntegrationTest and ParametrizedLenientInstrumentAppIntegrationTest The test cases are currently instantiated only with SYZYGY not with CLANG cause some fixes need to be done for the test cases to work with CLANG. BUG= Review-Url: https://codereview.chromium.org/2984303002 Committed: https://github.com/google/syzygy/commit/4c7cb860467e960396dc24e5146c57802ce3669b

Patch Set 1 #

Patch Set 2 : Removed parametrization with Clang until the necessary fixes are done. #

Total comments: 22

Patch Set 3 : Response to reviewers comments. #

Total comments: 5

Patch Set 4 : Response to reviewers comments. #

Patch Set 5 : Remove any CLANG code. #

Total comments: 10

Patch Set 6 : Response to reviewers comments #

Total comments: 2

Patch Set 7 : Response to reviewers comments #

Patch Set 8 : Response to reviewers comments #

Patch Set 9 : Rebase allocator shim #

Patch Set 10 : Fix merging issue. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+87 lines, -41 lines) Patch
M syzygy/integration_tests/instrument_integration_test.cc View 1 2 3 4 5 6 7 8 9 28 chunks +87 lines, -41 lines 0 comments Download

Messages

Total messages: 56 (39 generated)
njanevsk
Chris since Seb is OOO these two days I need your feedback on this CL.
3 years, 4 months ago (2017-07-27 15:51:25 UTC) #5
njanevsk
PTAL
3 years, 4 months ago (2017-07-31 19:04:25 UTC) #7
Sébastien Marchand
https://codereview.chromium.org/2984303002/diff/20001/syzygy/integration_tests/instrument_integration_test.cc File syzygy/integration_tests/instrument_integration_test.cc (left): https://codereview.chromium.org/2984303002/diff/20001/syzygy/integration_tests/instrument_integration_test.cc#oldcode883 syzygy/integration_tests/instrument_integration_test.cc:883: void AsanZebraHeapTest(bool enabled); Don't remove this function. https://codereview.chromium.org/2984303002/diff/20001/syzygy/integration_tests/instrument_integration_test.cc File ...
3 years, 4 months ago (2017-08-01 19:27:37 UTC) #12
njanevsk
I improved the design based on your comments. PTAL. https://codereview.chromium.org/2984303002/diff/20001/syzygy/integration_tests/instrument_integration_test.cc File syzygy/integration_tests/instrument_integration_test.cc (left): https://codereview.chromium.org/2984303002/diff/20001/syzygy/integration_tests/instrument_integration_test.cc#oldcode883 syzygy/integration_tests/instrument_integration_test.cc:883: ...
3 years, 4 months ago (2017-08-03 01:23:22 UTC) #19
Sébastien Marchand
https://codereview.chromium.org/2984303002/diff/20001/syzygy/integration_tests/instrument_integration_test.cc File syzygy/integration_tests/instrument_integration_test.cc (left): https://codereview.chromium.org/2984303002/diff/20001/syzygy/integration_tests/instrument_integration_test.cc#oldcode883 syzygy/integration_tests/instrument_integration_test.cc:883: void AsanZebraHeapTest(bool enabled); On 2017/08/03 01:23:21, njanevsk wrote: > ...
3 years, 4 months ago (2017-08-03 18:12:59 UTC) #22
Sébastien Marchand
https://codereview.chromium.org/2984303002/diff/20001/syzygy/integration_tests/instrument_integration_test.cc File syzygy/integration_tests/instrument_integration_test.cc (left): https://codereview.chromium.org/2984303002/diff/20001/syzygy/integration_tests/instrument_integration_test.cc#oldcode883 syzygy/integration_tests/instrument_integration_test.cc:883: void AsanZebraHeapTest(bool enabled); On 2017/08/03 18:12:59, Sébastien Marchand wrote: ...
3 years, 4 months ago (2017-08-03 20:44:31 UTC) #23
Sébastien Marchand
https://codereview.chromium.org/2984303002/diff/20001/syzygy/integration_tests/instrument_integration_test.cc File syzygy/integration_tests/instrument_integration_test.cc (left): https://codereview.chromium.org/2984303002/diff/20001/syzygy/integration_tests/instrument_integration_test.cc#oldcode883 syzygy/integration_tests/instrument_integration_test.cc:883: void AsanZebraHeapTest(bool enabled); On 2017/08/03 20:44:31, Sébastien Marchand wrote: ...
3 years, 4 months ago (2017-08-03 20:46:03 UTC) #24
njanevsk
PTAL.
3 years, 4 months ago (2017-08-07 15:47:01 UTC) #25
Sébastien Marchand
https://codereview.chromium.org/2984303002/diff/80001/syzygy/integration_tests/instrument_integration_test.cc File syzygy/integration_tests/instrument_integration_test.cc (right): https://codereview.chromium.org/2984303002/diff/80001/syzygy/integration_tests/instrument_integration_test.cc#newcode1292 syzygy/integration_tests/instrument_integration_test.cc:1292: class ParametrizedLenientInstrumentAppIntegrationTest Add a comment explaining the purpose of ...
3 years, 4 months ago (2017-08-07 16:47:00 UTC) #26
njanevsk
PTAL https://codereview.chromium.org/2984303002/diff/80001/syzygy/integration_tests/instrument_integration_test.cc File syzygy/integration_tests/instrument_integration_test.cc (right): https://codereview.chromium.org/2984303002/diff/80001/syzygy/integration_tests/instrument_integration_test.cc#newcode1292 syzygy/integration_tests/instrument_integration_test.cc:1292: class ParametrizedLenientInstrumentAppIntegrationTest On 2017/08/07 16:46:59, Sébastien Marchand wrote: ...
3 years, 4 months ago (2017-08-07 17:39:22 UTC) #27
Sébastien Marchand
lgtm https://codereview.chromium.org/2984303002/diff/100001/syzygy/integration_tests/instrument_integration_test.cc File syzygy/integration_tests/instrument_integration_test.cc (right): https://codereview.chromium.org/2984303002/diff/100001/syzygy/integration_tests/instrument_integration_test.cc#newcode1294 syzygy/integration_tests/instrument_integration_test.cc:1294: // Class created to enable parametrization of the ...
3 years, 4 months ago (2017-08-07 18:21:50 UTC) #28
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2984303002/120001
3 years, 4 months ago (2017-08-09 14:48:58 UTC) #31
commit-bot: I haz the power
Try jobs failed on following builders: win_dbg_try on master.tryserver.client.syzygy (JOB_FAILED, https://build.chromium.org/p/tryserver.client.syzygy/builders/win_dbg_try/builds/845)
3 years, 4 months ago (2017-08-09 14:51:26 UTC) #33
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2984303002/140001
3 years, 4 months ago (2017-08-09 17:07:55 UTC) #36
commit-bot: I haz the power
Try jobs failed on following builders: win_dbg_try on master.tryserver.client.syzygy (JOB_FAILED, https://build.chromium.org/p/tryserver.client.syzygy/builders/win_dbg_try/builds/846) win_rel_try on master.tryserver.client.syzygy (JOB_FAILED, ...
3 years, 4 months ago (2017-08-09 17:09:48 UTC) #38
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2984303002/180001
3 years, 4 months ago (2017-08-09 21:11:16 UTC) #53
commit-bot: I haz the power
3 years, 4 months ago (2017-08-09 21:11:30 UTC) #56
Message was sent while issue was closed.
Committed patchset #10 (id:180001) as
https://github.com/google/syzygy/commit/4c7cb860467e960396dc24e5146c57802ce3669b

Powered by Google App Engine
This is Rietveld 408576698