ContentVerifierTest.FailOnRead is Flaky |
|||||
Issue descriptionFindit has detected a flake at test ContentVerifierTest.FailOnRead. Culprit (85.3% confidence): https://chromium-review.googlesource.com/q/Icf3a5c8b4c0d01cb20e02de14b11aca4aeff03e5 Regression range: https://crrev.com/f9fdb14a14949b62e6b21aeb299766e4b4ea9e7d..daecbaf6c371c584547d1b8ac510b652614e2346?pretty=fuller Analysis: https://findit-for-me.appspot.com/waterfall/flake?key=ag9zfmZpbmRpdC1mb3ItbWVypgELEhdNYXN0ZXJGbGFrZUFuYWx5c2lzUm9vdCJwY2hyb21pdW0ubGludXgvTGludXggVGVzdHMgKGRiZykoMSkvNjk5MDcvbmV0d29ya19zZXJ2aWNlX2Jyb3dzZXJfdGVzdHMvUTI5dWRHVnVkRlpsY21sbWFXVnlWR1Z6ZEM1R1lXbHNUMjVTWldGawwLEhNNYXN0ZXJGbGFrZUFuYWx5c2lzGAEM If this result was incorrect, apply the label Findit-Incorrect-Result, mark the bug as Untriaged and the component Tools>Test>Findit>Flakiness.
,
Jan 30 2018
The failing DCHECK has been added in the identified CL https://chromium-review.googlesource.com/c/chromium/src/+/879961. Reverting the CL.
,
Jan 30 2018
The revert is https://chromium-review.googlesource.com/c/chromium/src/+/893258. Please have a look and reland.
,
Jan 30 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2181b01c2d22784987e7e166b97718c0d74a981a commit 2181b01c2d22784987e7e166b97718c0d74a981a Author: vitaliii <vitaliii@chromium.org> Date: Tue Jan 30 13:45:26 2018 Revert "Fix extraneous call to ContentVerifyJob::DispatchFailureCallback." This reverts commit 6e548ee1ca4829c87752ef04b2d97ad15a3ff3f9. Reason for revert: DCHECK at content_verify_job.cc:192 occasionally fails (see crbug.com/806586 ). Original change's description: > Fix extraneous call to ContentVerifyJob::DispatchFailureCallback. > > If ContentVerifyJob receives hashes (ContentVerifyJob::OnHashesReady) > after the content is read (ContentVerifyJob::DoneReading), then it > was possible for DispatchFailureCallback to be called more than once. > This causes a DCHECK failure in DispatchFailureCallback > (DCHECK(!failed_)). > > Bail out early in OnHashesReady if call to ContentVerifyJob::BytesRead > leads to a ContentVerifyJob failure. > > Note that this isn't harmful in Release builds as > DispatchFailureCallback tests ContentVerifyJob::failure_callback_ > before calling the callback, which is reset once it is called. > Nevertheless, this is incorrect behavior and this CL fixes that. > > > Bug: 804630 > Test: See bug description for repro steps. > Change-Id: Icf3a5c8b4c0d01cb20e02de14b11aca4aeff03e5 > Reviewed-on: https://chromium-review.googlesource.com/879961 > Reviewed-by: Devlin <rdevlin.cronin@chromium.org> > Commit-Queue: Istiaque Ahmed <lazyboy@chromium.org> > Cr-Commit-Position: refs/heads/master@{#531755} TBR=lazyboy@chromium.org,rdevlin.cronin@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 804630 , 806586 Change-Id: Ia5c67f60e95ddb607cb364c745f31dde465d3c4f Reviewed-on: https://chromium-review.googlesource.com/893258 Reviewed-by: vitaliii <vitaliii@chromium.org> Commit-Queue: vitaliii <vitaliii@chromium.org> Cr-Commit-Position: refs/heads/master@{#532854} [modify] https://crrev.com/2181b01c2d22784987e7e166b97718c0d74a981a/extensions/browser/content_verifier/content_hash.h [modify] https://crrev.com/2181b01c2d22784987e7e166b97718c0d74a981a/extensions/browser/content_verify_job.cc [modify] https://crrev.com/2181b01c2d22784987e7e166b97718c0d74a981a/extensions/browser/content_verify_job_unittest.cc [modify] https://crrev.com/2181b01c2d22784987e7e166b97718c0d74a981a/extensions/common/constants.cc [modify] https://crrev.com/2181b01c2d22784987e7e166b97718c0d74a981a/extensions/common/constants.h
,
Jan 30 2018
,
Jan 30 2018
Findit identified the culprit r531755 with confidence 80.5% in the config "chromium.linux / Linux Tests (dbg)(1)" based on the flakiness trend: https://findit-for-me.appspot.com/waterfall/flake?key=ag9zfmZpbmRpdC1mb3ItbWVypgELEhdNYXN0ZXJGbGFrZUFuYWx5c2lzUm9vdCJwY2hyb21pdW0ubGludXgvTGludXggVGVzdHMgKGRiZykoMSkvNjk5MDcvbmV0d29ya19zZXJ2aWNlX2Jyb3dzZXJfdGVzdHMvUTI5dWRHVnVkRlpsY21sbWFXVnlWR1Z6ZEM1R1lXbHNUMjVTWldGawwLEhNNYXN0ZXJGbGFrZUFuYWx5c2lzGAIM Automatically posted by the findit-for-me app (https://goo.gl/Ot9f7N). Flake Analyzer is in alpha version. Feedback is welcome using component Tools>Test>FindIt>Flakiness !
,
Jan 31 2018
I'm in the process of resubmitting the CL here after taking care of the issues. CL: https://chromium-review.googlesource.com/c/chromium/src/+/894824 The test in question was in the process of being removed b/c of orthogonal issues https://chromium-review.googlesource.com/c/chromium/src/+/888327
,
Jan 31 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/15f07945f50fd50f710cdb8a0191ac2a83050087 commit 15f07945f50fd50f710cdb8a0191ac2a83050087 Author: Istiaque Ahmed <lazyboy@chromium.org> Date: Wed Jan 31 05:15:03 2018 Revert "Revert "Fix extraneous call to ContentVerifyJob::DispatchFailureCallback."" This reverts commit 2181b01c2d22784987e7e166b97718c0d74a981a. Reason for revert: The offending test (ContentVerifierTest.FailOnRead) has been taken care of in CL: https://chromium-review.googlesource.com/c/chromium/src/+/888327 Original change's description: > Revert "Fix extraneous call to ContentVerifyJob::DispatchFailureCallback." > > This reverts commit 6e548ee1ca4829c87752ef04b2d97ad15a3ff3f9. > > Reason for revert: DCHECK at content_verify_job.cc:192 occasionally fails (see crbug.com/806586 ). > > Original change's description: > > Fix extraneous call to ContentVerifyJob::DispatchFailureCallback. > > > > If ContentVerifyJob receives hashes (ContentVerifyJob::OnHashesReady) > > after the content is read (ContentVerifyJob::DoneReading), then it > > was possible for DispatchFailureCallback to be called more than once. > > This causes a DCHECK failure in DispatchFailureCallback > > (DCHECK(!failed_)). > > > > Bail out early in OnHashesReady if call to ContentVerifyJob::BytesRead > > leads to a ContentVerifyJob failure. > > > > Note that this isn't harmful in Release builds as > > DispatchFailureCallback tests ContentVerifyJob::failure_callback_ > > before calling the callback, which is reset once it is called. > > Nevertheless, this is incorrect behavior and this CL fixes that. > > > > > > Bug: 804630 > > Test: See bug description for repro steps. > > Change-Id: Icf3a5c8b4c0d01cb20e02de14b11aca4aeff03e5 > > Reviewed-on: https://chromium-review.googlesource.com/879961 > > Reviewed-by: Devlin <rdevlin.cronin@chromium.org> > > Commit-Queue: Istiaque Ahmed <lazyboy@chromium.org> > > Cr-Commit-Position: refs/heads/master@{#531755} > > TBR=lazyboy@chromium.org,rdevlin.cronin@chromium.org > > # Not skipping CQ checks because original CL landed > 1 day ago. > > Bug: 804630 , 806586 > Change-Id: Ia5c67f60e95ddb607cb364c745f31dde465d3c4f > Reviewed-on: https://chromium-review.googlesource.com/893258 > Reviewed-by: vitaliii <vitaliii@chromium.org> > Commit-Queue: vitaliii <vitaliii@chromium.org> > Cr-Commit-Position: refs/heads/master@{#532854} TBR=lazyboy@chromium.org,rdevlin.cronin@chromium.org,vitaliii@chromium.org Change-Id: I82988d836f49033e1d618125e06f0d45d1124071 Bug: 804630 , 806586 Reviewed-on: https://chromium-review.googlesource.com/894824 Commit-Queue: Istiaque Ahmed <lazyboy@chromium.org> Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org> Cr-Commit-Position: refs/heads/master@{#533209} [modify] https://crrev.com/15f07945f50fd50f710cdb8a0191ac2a83050087/extensions/browser/content_verifier/content_hash.h [modify] https://crrev.com/15f07945f50fd50f710cdb8a0191ac2a83050087/extensions/browser/content_verify_job.cc [modify] https://crrev.com/15f07945f50fd50f710cdb8a0191ac2a83050087/extensions/browser/content_verify_job_unittest.cc [modify] https://crrev.com/15f07945f50fd50f710cdb8a0191ac2a83050087/extensions/common/constants.cc [modify] https://crrev.com/15f07945f50fd50f710cdb8a0191ac2a83050087/extensions/common/constants.h
,
Feb 1 2018
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by vitaliii@chromium.org
, Jan 30 2018