New issue
Advanced search Search tips

Issue 913421 link

Starred by 2 users
Status: Fixed
Owner:
jdarpinian@chromium.org
Closed: Dec 11
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression

Blocked on:
issue 905890


Sign in to add a comment

transfer_buffer_unittest.cc's new TransferBufferTest.ResizeDuringScopedResultPtr fails in official builds

Project Member Reported by thakis@chromium.org, Dec 10 
E.g. here: https://logs.chromium.org/logs/chromium/bb/chromium.clang/ToTWin/2780/+/recipes/steps/gpu_unittests/0/logs/TransferBufferTest.ResizeDuringScopedResultPtr/0 (from https://ci.chromium.org/buildbot/chromium.clang/ToTWin/2780).

[ RUN      ] TransferBufferTest.ResizeDuringScopedResultPtr
../../gpu/command_buffer/client/transfer_buffer_unittest.cc(756): error: Death test: transfer_buffer_->AllocUpTo(transfer_buffer_->GetFreeSize() + 1, &size_allocated)
    Result: died but not with expected error.
  Expected: outstanding_result_pointer_
Actual msg:


That's because official tests strip the string passed to CHECK(false) and LOG(FATAL), so you can't write death tests that check for a concrete string in official.

The usual fix is to not check for a string anywhere.


See  issue 779820  and the bug linked from there for prior art.

Comment 2 by jdarpinian@chromium.org, Dec 10

Status: Started (was: Untriaged)
Thanks for the heads up. We actually plan to change this CHECK to a DCHECK so that should fix the issue as well.

Comment 3 by jdarpinian@chromium.org, Dec 10

Blockedon: 905890
Project Member

Comment 4 by bugdroid1@chromium.org, Dec 11 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9c995a82acfcfc47c8650c5edea8e5e6c28d7893

commit 9c995a82acfcfc47c8650c5edea8e5e6c28d7893
Author: James Darpinian <jdarpinian@chromium.org>
Date: Tue Dec 11 22:00:24 2018

gpu: Change CHECK to DCHECK in transfer buffer

As requested in the review of https://crrev.com/c/1336753. After
https://crbug.com/913996 is fixed we can remove
outstanding_result_pointer_ entirely when DCHECKs are disabled.

It also turns out that we can't use the string argument to ASSERT_DEATH
to detect a CHECK message in a test because the message strings are
stripped in official builds.

Bug: 905890,  913421 , 913996
Change-Id: Ia7df19057564ef7a4bbbab9ba36f583c6e6bca0f
Reviewed-on: https://chromium-review.googlesource.com/c/1370484
Commit-Queue: James Darpinian <jdarpinian@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615680}
[modify] https://crrev.com/9c995a82acfcfc47c8650c5edea8e5e6c28d7893/gpu/command_buffer/client/transfer_buffer.cc
[modify] https://crrev.com/9c995a82acfcfc47c8650c5edea8e5e6c28d7893/gpu/command_buffer/client/transfer_buffer_unittest.cc

Comment 5 by jdarpinian@chromium.org, Dec 11

Status: Fixed (was: Started)
I ran into bug 913996 while fixing this.
►

Sign in to add a comment