New issue
Advanced search Search tips

Issue 791211 link

Starred by 2 users

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Eliminate as many reasons to disable the "deapply patch" step in trybots as possible.

Project Member Reported by dpranke@chromium.org, Dec 2 2017

Issue description

Currently the chromium trybot recipe attempts to handle failures on the main waterfalls by comparing the failures that arise in a tryjob with the current patch against the failures that happen if the patch is deapplied; if the same failure happens both with and without the patch, we decide that the failure isn't the patch's fault and ignore it for purposes of determining if the tryjob fails.

However, there are a class of changes for which we *don't* do this, i.e., the failure is considered fatal and we don't even try to deapply the patch. I think the original thinking for this was that we thought we couldn't figure out what to re-run because the configuration had changed. In other words, if we add a new test step as part of the patch, and then de-applied the patch, we weren't smart enough to figure out that we didn't need to run the step for comparison.

Given that not being able to de-apply the patch means that a class of CL will then fail tryjobs for reasons that aren't the CL's fault, this is bad, and we should try to make this happen as rarely as possible (ideally, never).
 
Status: Available (was: Untriaged)
Links to the relevant code:

https://cs.chromium.org/chromium/build/scripts/slave/recipe_modules/chromium_tests/api.py?l=20
https://cs.chromium.org/chromium/build/scripts/slave/recipe_modules/chromium_tests/api.py?https://cs.chromium.org/chromium/src/testing/buildbot/

I think we have the following cases:

1) PRESUBMIT.py and trybot_analyze_config.json, which should be safe to ignore.
2) gn_isolate_map.pyl, the filters/* files, and the *.json files, which may change how the test is run but which should be safe to retry in the cases where the test step isn't being added as part of the CL.

Fixing (1) should just involve making the api.py logic slightly smarter.

Fixing (2) likely means that we have to regenerate the test step objects when we deapply the patch and compare them against the original test step objects, to see if the steps are no longer necessary.
Issue 791038 has been merged into this issue.
Project Member

Comment 4 by bugdroid1@chromium.org, Dec 5 2017

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

commit 05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8
Author: Dirk Pranke <dpranke@chromium.org>
Date: Tue Dec 05 23:17:58 2017

Clean up handling of test args and --gtest_filter, esp. on retries.

The interaction between --gtest_filter and --test-launcher-filter-file
is somewhat complicated, and the recipe-side code was making it even
more complicated. In particular, in cases where a test was configured
to use a filter file, the recipe would throw away the filter file
argument on a retry, and that would cause problems when we changed
the filter files as part of a patch.

This CL rationalizes the changes a bit, so that we always use
--gtest-filter for the failures to retry; eventually, we should
change this so that we always use --test-launcher-filter-file so
that we don't have to worry about the size of the command line, but
that requires more work to ensure that the file is plumbed through
to swarming properly.

In addition, this removes an argument that is apparently no longer
needed in the chromium_android API, making the control flow slightly
simpler (one fewer place to override things), and removes the use
of a static method from the GTestTest class; really the whole class
needs to be removed, but that's for a different CL.

R=jbudorick@chromium.org
BUG=667004, 791211

Change-Id: I1d5899cbbbb2eeee72aba058f87d73bb8b02cb76
Reviewed-on: https://chromium-review.googlesource.com/804682
Commit-Queue: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: John Budorick <jbudorick@chromium.org>

[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/asan_basic.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/tester_other_device_failure_during_recovery.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/upload_result_details_failures.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipes/chromium_trybot.expected/invalid_results.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/perf_runner_basic.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipes/findit/chromium/test.expected/flaky_tests.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipes/findit/chromium/test.expected/remove_culprits_for_flaky_failures.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/asan_setup_failure.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/tester_blacklisted_devices.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipes/findit/chromium/test.expected/all_test_failed.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/gerrit_try_builder_basic.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/tester_with_step_warning.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/keep_data_install_tester_basic.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/perf_runner_disable_location_basic.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/downgrade_install_tester_basic.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipes/findit/chromium/flake.expected/use_build_parameter_for_tests.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipes/findit/chromium/test.expected/unaffected_test_skipped_by_analyze.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipes/findit/chromium/test.expected/use_abbreviated_revision_in_step_name.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/tester_failing_host_info.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/api.py
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/perf_tests_infra_reference_failure.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/perf_runner_allow_high_battery_temp_basic.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/webview_cts_basic.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/no_cache_builder_basic.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipes/findit/chromium/test.expected/use_build_parameter_for_tests_non_json_buildbucket.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipes/findit/chromium/test.expected/all_test_passed.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/gerrit_refs.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/telemetry_browser_tests_failures.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipes/findit/chromium/test.expected/findit_consecutive_culprits.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/tester_no_devices_during_status.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipes/findit/chromium/test.expected/findit_culprit_in_last_sub_range.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/tester_other_device_failure_during_status.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipes/findit/chromium/test.expected/only_one_test_passed.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_tests/tests/steps/gtest_test.expected/basic.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/basic_builder_basic.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/result_details_basic.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipes/findit/chromium/test.expected/none_swarming_tests.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/coverage_builder_basic.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_tests/tests/steps/json_results_handler.expected/invalid.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/tester_offline_devices.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/tester_no_devices_during_recovery.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/perf_tests_infra_failure.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipes/findit/chromium/test.expected/findit_steps_multiple_culprits.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/slow_tester_basic.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_tests/tests/steps/local_gtest_test.expected/android.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipes/findit/chromium/flake.expected/flakiness_swarming_tests.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipes/findit/chromium/flake.expected/flakiness_non-swarming_tests.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/restart_usb_builder_basic.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipes/findit/chromium/test.expected/findit_tests_multiple_culprits.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_tests/steps.py
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/perf_runner_user_build_basic.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipes/chromium.expected/dynamic_local_isolated_script_test_with_corrupt_json_results.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/perf_tests_failure.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipes/chromium.expected/dynamic_swarmed_sharded_corrupt_json_isolated_script_test.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipes/findit/chromium/test.expected/findit_culprit_in_first_sub_range.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/tester_basic.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/timestamp_as_point_id_basic.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipes/findit/chromium/test.expected/use_build_parameter_for_tests.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/upload_archives_to_bucket_basic.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/remove_system_vrcore_basic.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/stackwalker_basic.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipes/chromium.expected/dynamic_isolated_script_test_harness_failure_no_json.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/perf_tests_reference_failure.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/telemetry_browser_tests_tester_basic.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/json_results_file_basic.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/no_strict_mode_tester_basic.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/resource_size_builder_basic.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/perf_adb_vendor_keys_basic.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/device_flags_builder_basic.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.py
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/tombstones_m53.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/README.recipes.md
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/webview_tester_basic.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/perf_runner_allow_low_battery_basic.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipes/findit/chromium/test.expected/use_analyze_set_to_False_for_non_linear_try_job.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipes/findit/chromium/test.expected/swarming_tests.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_tests/tests/steps/local_gtest_test.expected/basic.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipe_modules/chromium_android/examples/full.expected/use_devil_adb_basic.json
[modify] https://crrev.com/05f3d5805c745ea4b0f995cdd4ca73b9c027c3a8/scripts/slave/recipes/findit/chromium/test.expected/findit_culprit_in_middle_sub_range.json

Project Member

Comment 5 by sheriffbot@chromium.org, Dec 6

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
I agree that we should implement c#2, (2). martiniss, given that you've been looking at  the Test subclasses recently, is this something you would be interested in taking on?
Labels: -Pri-1 Pri-2
Status: Available (was: Untriaged)
This would certainly still be useful, but I doubt anyone has the cycles for it atm. Back into our backlog.

Sign in to add a comment