New issue
Advanced search Search tips

Issue 892428 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 30
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Build Recipes Tester should test without patch, retry summary code paths

Project Member Reported by martiniss@chromium.org, Oct 4

Issue description

Currently, the Build Recipes Tester doesn't test all of the code paths in the chromium_trybot recipe. Specifically, it doesn't do much with patches, and their deapplication.

Right now, it runs with no patch. If any test fails (often due to flake), the led task throws an exception because of a failed assertion. This does potentially lead to more flake than necessary, since the build fails before it can run the tests without the patch.

It's unclear what patch, if any, the build should run with. Potentially we should just give it a no-op patch? I don't know if it'd make sense to do that, and I'm a bit worried about relying on a particular CL always being around.

Thoughts?
 
Cc: martiniss@chromium.org
 Issue 892262  has been merged into this issue.
One idea is to make a patch which touches all the different types of tests we support. So, modify webkit_layout_tests, a gtest based test, maybe a few others. Would probably just be a whitespace change.

This would test all the different kinds of test suites, without running every single test.

We theoretically could also test that a patch we know should make a CQ job fail does. Although maybe that should be a separate builder. Maybe we'd trigger that from the same build, and just trigger two different jobs in the same build.
 Issue 896420  has been merged into this issue.
FWIW, when I want to run a tryjob that triggers *everything*, I just do a whitespace change to an analyze exclusion file:
https://chromium.googlesource.com/chromium/src/+/master/testing/buildbot/trybot_analyze_config.json

eg: https://chromium-review.googlesource.com/c/chromium/src/+/1187550

I know those exist.

I'm wondering if I want to trigger everything. That'll just make the build take longer, and I'd like this to be as fast as possible, since users are used to the build CQ being very short. That's why I only want to trigger a select few tests. Maybe that's too complicated and I should just trigger everything.
Cc: iannu...@google.com
Cc: -iannucci@chromium.org
Project Member

Comment 8 by bugdroid1@chromium.org, Oct 29

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/1d22e570204231a01d2d96fc6d8429cfa49ff93c

commit 1d22e570204231a01d2d96fc6d8429cfa49ff93c
Author: Stephen Martinis <martiniss@chromium.org>
Date: Mon Oct 29 23:51:16 2018

led_recipe_tester: Apply a patch

This CL makes the led recipe tester apply a patch when running tests.
This exercise more code paths, and fix weird error messages when a test
fails in the CL due to it being flaky.

Bug:  892428 
Change-Id: Ic5986d1d286343aec9ebc98d2fb65bbf4311a525
Reviewed-on: https://chromium-review.googlesource.com/c/1289732
Reviewed-by: Ben Pastene <bpastene@chromium.org>
Reviewed-by: John Budorick <jbudorick@chromium.org>
Commit-Queue: Stephen Martinis <martiniss@chromium.org>

[modify] https://crrev.com/1d22e570204231a01d2d96fc6d8429cfa49ff93c/scripts/slave/README.recipes.md
[modify] https://crrev.com/1d22e570204231a01d2d96fc6d8429cfa49ff93c/scripts/slave/recipes/led_recipes_tester.expected/basic.json
[modify] https://crrev.com/1d22e570204231a01d2d96fc6d8429cfa49ff93c/scripts/slave/recipes/led_recipes_tester.py

Status: Fixed (was: Assigned)
Marking as fixed for now. We can revisit this later, but for now it's applying a patch to DEPS.

Sign in to add a comment