New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 774689 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

CL with 100% unittest failure passed the PreCQ

Project Member Reported by dgarr...@chromium.org, Oct 13 2017

Issue description

This CL (at Rev 2) passed the PreCQ.

https://crrev.com/c/713463

However, it contained an error that would always cause update_engine unittests to fail.

A sample build:

https://luci-milo.appspot.com/buildbot/chromiumos.tryserver/no_vmtest_pre_cq/145389
 
Just a correction, update_payload unittests fail (not update_engine).

Comment 2 Deleted

Oops... which means pulling all of that together was useless. ;>

Version during build "update_payload-0.0.1-r8.ebuild"

Uprev stage:

Marking 9999 ebuild for chromeos-base/update_payload as stable.

BuildPackages:

update_payloads not mentioned.

Unittest Stage:

update_payloads not mentioned.

19:33:16: INFO: RunCommand: sudo 'PARALLEL_EMERGE_STATUS_FILE=/tmp/tmpLMH_Be' 'FEATURES=test' 'PKGDIR=/build/daisy_spring/test-packages' -- /mnt/host/source/chromite/bin/parallel_emerge '--sysroot=/build/daisy_spring' '--jobs=10' sys-apps/rootdev chromeos-base/imageloader chromeos-base/libbrillo chromeos-base/vboot_reference chromeos-base/p2p chromeos-base/chaps chromeos-base/chromite chromeos-base/easy-unlock chromeos-base/google-breakpad chromeos-base/permission_broker chromeos-base/chromeos-config-tools chromeos-base/chromeos-ec chromeos-base/shill net-misc/modemmanager-next chromeos-base/chromeos-login chromeos-base/vpn-manager chromeos-base/chromeos-trim chromeos-base/cromo chromeos-base/chromeos-init chromeos-base/bootstat sys-apps/flashmap chromeos-base/libcontainer chromeos-base/quipper dev-util/puffin chromeos-base/mist net-wireless/bluez chromeos-base/libchromeos-ui sys-apps/portage chromeos-base/firewalld chromeos-base/chromeos-minijail media-sound/adhd chromeos-base/vpd chromeos-base/easy-unlock-crypto net-libs/libmbim chromeos-base/wimax_manager chromeos-base/gestures chromeos-base/crash-reporter chromeos-base/debugd chromeos-base/power_manager chromeos-base/chromeos-imageburner chromeos-base/autotest chromeos-base/cryptohome chromeos-base/verity sys-apps/mosys chromeos-base/update_engine dev-util/bsdiff chromeos-base/factory_installer chromeos-base/chromeos-installer chromeos-base/mtpd chromeos-base/metrics sys-apps/flashrom chromeos-base/crosh chromeos-base/cros-disks chromeos-base/userfeedback

So, we applied the change, locally upreved, but did not build or test.
Cc: vapier@chromium.org davidjames@chromium.org
Labels: -Pri-3 Pri-1
Deleted bad data to avoid confusion.
The problem appears to be in cros_run_unit_tests, which hasn't meaningfully changed since 2015.

19:33:10: INFO: RunCommand: /b/c/cbuild/repository/chromite/bin/cros_sdk 'USE=chrome_internal' 'PARALLEL_EMERGE_STATUS_FILE=/tmp/tmpLMH_Be' -- /mnt/host/source/chromite/bin/cros_run_unit_tests '--board=daisy_spring' in /b/c/cbuild/repository


Since a board is passed in, but no package list, I think this is the relevant logic for selecting packages to test:

  # If no packages were specified, use all testable packages.
  if not (opts.packages or opts.package_file):
    workon = workon_helper.WorkonHelper(sysroot)
    packages = (workon.InstalledWorkonAtoms() if opts.installed
                else workon.ListAtoms(use_all=True))

Comment 9 by vapier@chromium.org, Oct 13 2017

chromeos-base/update_payload is only depended on by chromeos-base/devserver-deps

chromeos-base/devserver-deps is only depended on by chromeos-base/devserver and chromeos-base/cros-devutils

chromeos-base/cros-devutils is only depended on by the sdk and chromeos-base/cros-testutils (which is also only the sdk)

chromeos-base/devserver is only depended on by moblab boards and lakitu's test image

we run unittests for target packages, not for sdk packages

so i'm not seeing a bug in cbuildbot here, just inadequate precq coverage
cros_run_unit_tests looks fine to me (especially after comment #9 context)
workon_helper seems to be touched most often by vapier@ and briannorris@.

Does either of you feel like the right person to figure this out?
Pre-Commit queue for Rev 2 just failed. Why commit queue was attempted before precq finishes?
never mind, I got confused. It was the actual CQ failure notice. not precq.
Rev 2 was already in the CQ when jclinton marked it as -1. The rev 2 CQ run failed, but the Rev 3 PreCQ is still running.
What would all be a lot easier to see if "Where is my CL" was finished.

Chatted with vapier@ and I'm dithering between adding a general moblab PreCQ builder, and adding one to COMMIT_QUEUE.ini for this repo.

Either would have caught this failure.
imo moblab should be part of the default precq rotation.  it's drastically different from our standard chromebook baseline.
Conceptually, I agree, but we are a little resource constrained for PreCQ builders (this temporary), doing it per-repo for now.
Owner: dgarr...@chromium.org
Status: Assigned (was: Untriaged)
Per Comment #15, assigning to dgarrett@.
Project Member

Comment 19 by bugdroid1@chromium.org, Oct 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/aosp/platform/system/update_engine/+/a6f74d62033fb0291aa1d135fef640e26ffffab5

commit a6f74d62033fb0291aa1d135fef640e26ffffab5
Author: Don Garrett <dgarrett@google.com>
Date: Sat Oct 14 19:37:50 2017

COMMIT-QUEUE.ini: Create an INI and add moblab.

The update_payloads ebuild is part of the update_engine repository,
but is used as part of the devserver code, so we only run it's
unittests for a small number of boards. Include one of those boards
here so we can catch problems before they hit the CQ.

BUG= chromium:774689 
TEST=None

Change-Id: I5888ab3c5218fd12206d2fb3291d900d7f4421f0
Reviewed-on: https://chromium-review.googlesource.com/719772
Commit-Ready: Don Garrett <dgarrett@chromium.org>
Tested-by: Don Garrett <dgarrett@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[add] https://crrev.com/a6f74d62033fb0291aa1d135fef640e26ffffab5/COMMIT-QUEUE.ini

Status: Fixed (was: Assigned)
I filed https://crbug.com/774732 to cover thinking away a way to automatically check for proper PreCQ coverage for all packages.

Calling this fixed.

Sign in to add a comment