New issue
Advanced search Search tips

Issue 596566 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Apr 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocking:
issue 591633



Sign in to add a comment

chromiumos.chromium *-tot-asan-informational builders should fail clearly instead of timing out

Project Member Reported by steve...@chromium.org, Mar 21 2016

Issue description

Currently when an ASAN builder on the chromiumos.chromium waterfall fails due to an ASAN error the builder times out with no obvious indication of the actual failure.

We should expose the actual failure cause to the build page.

Example failure:  issue 595068 
 
It appears that unit tests run via the ebuild FEATURES=test flag. Then ebuilds which support the test feature flag get executed. It's hard to find documentation on this, so it could very well be wrong.

So each unit test is a separate binary. For example, here[1] is the file that runs the update_engine unit tests, which is what caused the failure in this scenario. The failure happened because the update_engine unittest opened up a subprocess that ASAN killed. There were no captured logs for this subprocess available in the build report page.

I think the right thing to do is to find a way to capture the stdout/stderr of subprocesses, but I haven't figured out a way to do it such that it automatically works on every unittest.

1: https://android.git.corp.google.com/platform/system/update_engine/+/260f03bc4d3a3de436e056c686c814444358823a/run_unittests
Looking more closely, I think we actually *are* including the logs from the subprocess. It's just that the ASAN crash logs are not being reported.

There is also the issue where deymo mentions here[1], which is that the test execution framework doesn't properly kill child processes.

I also found some important files w.r.t. how unit tests get executed:
- platform.eclass https://chromium.git.corp.google.com/chromiumos/overlays/chromiumos-overlay/+/master/eclass/platform.eclass
- platform2_test.py https://chromium.git.corp.google.com/chromiumos/platform2/+/master/common-mk/platform2_test.py
- For the update engine in particular, https://chromium.git.corp.google.com/chromiumos/overlays/chromiumos-overlay/+/master/chromeos-base/update_engine/update_engine-9999.ebuild

1: https://bugs.chromium.org/p/chromium/issues/detail?id=595068#c4
https://chromium-review.googlesource.com/#/c/335250 should help deal with the timeout issue.
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 5 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/0feab3498e8c4c9e4fc1d8a5b8a2178418cfc459

commit 0feab3498e8c4c9e4fc1d8a5b8a2178418cfc459
Author: Jacob Dufault <jdufault@google.com>
Date: Thu Mar 31 19:10:46 2016

Add psutil dependency for platform2_test.py

BUG= chromium:596566 
TEST=cros_run_unit_tests --board link --packages="update_engine"

Change-Id: I15666c3d87ad58237df491b21fb8c1486f900a17
Reviewed-on: https://chromium-review.googlesource.com/336456
Commit-Ready: Jacob Dufault <jdufault@chromium.org>
Tested-by: Jacob Dufault <jdufault@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[rename] https://crrev.com/0feab3498e8c4c9e4fc1d8a5b8a2178418cfc459/virtual/target-chromium-os-sdk/target-chromium-os-sdk-1-r61.ebuild
[modify] https://crrev.com/0feab3498e8c4c9e4fc1d8a5b8a2178418cfc459/virtual/target-chromium-os-sdk/target-chromium-os-sdk-1.ebuild

Project Member

Comment 5 by bugdroid1@chromium.org, Apr 7 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/fd24e9b9796336cb7506e17e8719b9395ec308bc

commit fd24e9b9796336cb7506e17e8719b9395ec308bc
Author: Jacob Dufault <jdufault@google.com>
Date: Tue Apr 05 17:16:13 2016

common-mk: Kill any auxiliary child processes after the child terminates.

It's possible the child will fork but the forked process will continue running
even after the child exits. This leads to hard-to-debug timeout errors since the
test never finishes.

BUG= chromium:596566 
TEST=cros_run_unit_tests --board link --packages="update_engine"
TEST=manually verified pgid behavior in repl

Change-Id: Ie483e07a24dbf4e1e176b9dee8479c7d35a5cafe
Reviewed-on: https://chromium-review.googlesource.com/335250
Commit-Ready: Jacob Dufault <jdufault@chromium.org>
Tested-by: Jacob Dufault <jdufault@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/fd24e9b9796336cb7506e17e8719b9395ec308bc/common-mk/platform2_test.py

Status: Fixed (was: Assigned)
Labels: VerifyIn-51
Project Member

Comment 8 by bugdroid1@chromium.org, Apr 21 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/6a4bc272a5904a7942682404f2dd7959a2a15cc5

commit 6a4bc272a5904a7942682404f2dd7959a2a15cc5
Author: Jacob Dufault <jdufault@google.com>
Date: Tue Apr 19 19:54:17 2016

common-mk: Some style fixes for platform2_test.py

BUG= chromium:596566 
TEST=cros_run_unit_tests --board link

Change-Id: Id55fa93d7258af997ecb2d4fcd0eacbe432011f2
Reviewed-on: https://chromium-review.googlesource.com/339711
Commit-Ready: Jacob Dufault <jdufault@chromium.org>
Tested-by: Jacob Dufault <jdufault@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/6a4bc272a5904a7942682404f2dd7959a2a15cc5/common-mk/platform2_test.py

Project Member

Comment 9 by bugdroid1@chromium.org, Apr 21 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/ddeba19e6811cc199a3109d908336dd02c759425

commit ddeba19e6811cc199a3109d908336dd02c759425
Author: Jacob Dufault <jdufault@google.com>
Date: Tue Apr 19 19:49:48 2016

vpn: Ensure child process is killed before exiting.

The UnitTest stage is becoming more strict; any auxiliary processes need to be
terminated before the test process exits, otherwise the test will fail.

BUG= chromium:596566 
TEST=cros_run_unit_tests --board link

Change-Id: I582df3a6d388a4993a5cad937bf2f0b2a98aaf0a
Reviewed-on: https://chromium-review.googlesource.com/339741
Commit-Ready: Jacob Dufault <jdufault@chromium.org>
Tested-by: Jacob Dufault <jdufault@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/ddeba19e6811cc199a3109d908336dd02c759425/vpn-manager/daemon_test.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Apr 21 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/b5da95305a1c4d0e92528a013b05ad9e36ff7bd0

commit b5da95305a1c4d0e92528a013b05ad9e36ff7bd0
Author: Jacob Dufault <jdufault@google.com>
Date: Tue Apr 19 19:45:06 2016

libchromeos-ui: Ensure the server process is killed before exiting.

The UnitTest stage is now more strict and all child processes need to be
terminated before the test completes, otherwise the test will fail.

BUG= chromium:596566 
TEST=cros_run_unit_tests --board link

Change-Id: Icf25426c03d411e44256f0839384ed8bc9bf20c5
Reviewed-on: https://chromium-review.googlesource.com/339721
Commit-Ready: Jacob Dufault <jdufault@chromium.org>
Tested-by: Jacob Dufault <jdufault@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/b5da95305a1c4d0e92528a013b05ad9e36ff7bd0/libchromeos-ui/chromeos/ui/x_server_runner_unittest.cc
[modify] https://crrev.com/b5da95305a1c4d0e92528a013b05ad9e36ff7bd0/libchromeos-ui/chromeos/ui/x_server_runner.cc

Project Member

Comment 11 by bugdroid1@chromium.org, Apr 22 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/90ed00fd305236a1acbbf2e8189f257228189530

commit 90ed00fd305236a1acbbf2e8189f257228189530
Author: Jacob Dufault <jdufault@google.com>
Date: Tue Apr 12 17:47:14 2016

common-mk: Kill any auxiliary child processes after the child terminates.

It's possible the child will fork but the forked process will continue running
even after the child exits. This leads to hard-to-debug timeout errors since the
test never finishes.

BUG= chromium:596566 
TEST=cros_run_unit_tests --board link --packages="update_engine"
TEST=manually verified process behavior in repl

Change-Id: I5b60d52145f4080dcae46c6113b9e18cd006b1ee
Reviewed-on: https://chromium-review.googlesource.com/338452
Commit-Ready: Jacob Dufault <jdufault@chromium.org>
Tested-by: Jacob Dufault <jdufault@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/90ed00fd305236a1acbbf2e8189f257228189530/common-mk/platform2_test.py

Status: Verified (was: Fixed)
Bulk verified
Project Member

Comment 13 by bugdroid1@chromium.org, Jan 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/c41740c5d6afc1240285a5fb4fdf931a560d2917

commit c41740c5d6afc1240285a5fb4fdf931a560d2917
Author: Jacob Dufault <jdufault@google.com>
Date: Tue Apr 12 17:47:14 2016

common-mk: Kill any auxiliary child processes after the child terminates.

It's possible the child will fork but the forked process will continue
running even after the child exits.  This leads to hard-to-debug timeout
errors since the test never finishes.

This version doesn't make the leaked processes a failure.  We want the
debug info more than being super strict here.

BUG= chromium:596566 
BUG= chromium:678643 
TEST=cros_run_unit_tests --board link --packages="update_engine"
TEST=manually verified process behavior in repl

Change-Id: Iab8652118ffd17be6913330bbfcc8fd6f91fc711
Reviewed-on: https://chromium-review.googlesource.com/428038
Commit-Ready: Ben Chan <benchan@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Ben Chan <benchan@chromium.org>

[modify] https://crrev.com/c41740c5d6afc1240285a5fb4fdf931a560d2917/common-mk/platform2_test.py

Sign in to add a comment