New issue
Advanced search Search tips

Issue 747183 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Jul 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Some "no-vmtest" build configs run VM tests

Reported by jrbarnette@chromium.org, Jul 21 2017

Issue description

A small handful of builders that you would think don't run
VM tests are nonetheless configured to run them:
    cyan-no-vmtest-pre-cq
    novato-no-vmtest-pre-cq
    amd64-generic-cheets-no-vmtest-pre-cq
    newbie-no-vmtest-pre-cq
    betty-no-vmtest-pre-cq

This was discovered in the process of debugging  bug 747167 ...

 
This appears to be the root cause:

# List of boards that run VMTests but only the smoke tests, not the AU tests
# until b/31341543 has been fixed.
_smoke_only_vmtest_boards = frozenset([
    'amd64-generic-cheets',
    'betty',
    'cyan',
    'newbie',
    'novato',
])

def CreateBoardConfigs(site_config, boards_dict, ge_build_config):
  # ...
    if board in _smoke_only_vmtest_boards:
      board_config.apply(site_config.templates.smoke_only_vmtest_builder)
  # ...

The `if` test doesn't exclude builders that disable VM tests...

Project Member

Comment 2 by bugdroid1@chromium.org, Jul 26 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/87739182e89627522b0ad836e357361a12548456

commit 87739182e89627522b0ad836e357361a12548456
Author: Don Garrett <dgarrett@google.com>
Date: Wed Jul 26 09:07:31 2017

chromeos_config: Verify no boards define tests.

When we define which tests to run at the board level, this fully
overrides the tests for all builders, which isn't what we want. For
example, this can cause us to run vmtests on a
<board>-no-vmtests-pre-cq build config.

Add unittests to find when this is added and warn people.

Turning tests OFF at the board level is something we do regularly, if
(for example) a VMTests are not supported for a given board (like
ARM).

BUG= chromium:747183 
TEST=Unittests

Change-Id: I9b29c0c7860ea515f58089c195497439f42d7c94
Reviewed-on: https://chromium-review.googlesource.com/581794
Commit-Ready: Don Garrett <dgarrett@chromium.org>
Tested-by: Don Garrett <dgarrett@chromium.org>
Reviewed-by: Don Garrett <dgarrett@chromium.org>
Reviewed-by: Bernie Thompson <bhthompson@chromium.org>

[modify] https://crrev.com/87739182e89627522b0ad836e357361a12548456/cbuildbot/chromeos_config_unittest.py

Project Member

Comment 3 by bugdroid1@chromium.org, Jul 26 2017

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

commit cd9df36626bcdb78ba488beb6183b099e424affd
Author: Don Garrett <dgarrett@google.com>
Date: Wed Jul 26 09:07:31 2017

chromeos_config: Remove smoke_only_vmtest concept.

As a workaround for b/31341543, a smoke_only_vmtest mechanism was
added. This has later (incorrectly) started adding tests to no_vmtest
and compile_only builders. Remove this mechanism, since b/31341543 is
marked fixed.

BUG= chromium:747183 
TEST=Unittests, betty-release tryjob.

Change-Id: I3c24cdb182c4f3713b11e0d9e2b4edb2930056f3
Reviewed-on: https://chromium-review.googlesource.com/581935
Commit-Ready: Don Garrett <dgarrett@chromium.org>
Tested-by: Don Garrett <dgarrett@chromium.org>
Reviewed-by: Nicolas Norvez <norvez@chromium.org>
Reviewed-by: Don Garrett <dgarrett@chromium.org>

[modify] https://crrev.com/cd9df36626bcdb78ba488beb6183b099e424affd/cbuildbot/config_dump.json
[modify] https://crrev.com/cd9df36626bcdb78ba488beb6183b099e424affd/cbuildbot/waterfall_layout_dump.txt
[modify] https://crrev.com/cd9df36626bcdb78ba488beb6183b099e424affd/cbuildbot/chromeos_config.py

Owner: dgarr...@chromium.org
Status: Verified (was: Untriaged)
I added this unit test temporarily:
diff --git a/cbuildbot/chromeos_config_unittest.py b/cbuildbot/chromeos_config_unittest.py
index f0c9b6f7..89d104fb 100644
--- a/cbuildbot/chromeos_config_unittest.py
+++ b/cbuildbot/chromeos_config_unittest.py
@@ -438,6 +438,13 @@ class CBuildBotTest(ChromeosConfigTestBase):
             config_lib.IsPFQType(config['build_type']),
             'Config %s: has chrome_rev but is not a PFQ.' % build_name)
 
+  def testNoVMTestHasNoVMTest(self):
+    problems = set()
+    for build_name, config in self.site_config.iteritems():
+      if 'no-vmtest' in build_name and config['vm_tests']:
+        problems.add(build_name)
+    self.assertSetEqual(problems, set())
+
   def testValidVMTestType(self):
     """Verify vm_tests has an expected value"""
     for build_name, config in self.site_config.iteritems():

The test failed before, when I filed the bug; now it passes.

Nothing wrong with that test either.

Sign in to add a comment