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

Issue 909834 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

'retry with patch' should only retry failing tests 10 times, not 40 times.

Project Member Reported by erikc...@chromium.org, Nov 28

Issue description

We set --gtest_repeat=10 for 'retry with patch' when retrying failed tests. Unfortunately, the gtest harness continues to use the default --test-launcher-retry-limit=4, which means that if the test fails on every run, it will actually be repeated 40 times.
 
Cc: nednguyen@chromium.org
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 28

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

commit 803f9d5158ec63ecbb8f73ec6b5fcfb852e2d9c4
Author: erikchen <erikchen@chromium.org>
Date: Wed Nov 28 21:23:50 2018

'retry with patch' should only rerun failing tests 10 times.

'retry with patch' uses --gtest_repeat=10 to rerun failing tests 10 times.
Unfortunately, the default value of --test-launcher-retry-limit=4 causes
deterministically failing tests to be rerun 40 times. This is not the intention.

This CL updates the recipe logic so that if we set --gtest_repeat=10, then we
also set --test-launcher-retry-limit=1.

Bug: 909834
Change-Id: I80b7972d7a08d161b8fa2a1f2c9e3f6c3af6a1a5
Reviewed-on: https://chromium-review.googlesource.com/c/1351695
Commit-Queue: Erik Chen <erikchen@chromium.org>
Reviewed-by: John Budorick <jbudorick@chromium.org>

[modify] https://crrev.com/803f9d5158ec63ecbb8f73ec6b5fcfb852e2d9c4/scripts/slave/recipes/chromium_trybot.expected/invalid_results.json
[modify] https://crrev.com/803f9d5158ec63ecbb8f73ec6b5fcfb852e2d9c4/scripts/slave/recipe_modules/chromium_tests/tests/api/run_tests_on_tryserver.expected/enable_retry_with_patch_succeed_after_deapply.json
[modify] https://crrev.com/803f9d5158ec63ecbb8f73ec6b5fcfb852e2d9c4/scripts/slave/recipe_modules/chromium_tests/tests/api/run_tests_on_tryserver.expected/enable_retry_with_patch_invalid_test_results.json
[modify] https://crrev.com/803f9d5158ec63ecbb8f73ec6b5fcfb852e2d9c4/scripts/slave/recipes/chromium_trybot.expected/swarmed_webkit_tests_interrupted.json
[modify] https://crrev.com/803f9d5158ec63ecbb8f73ec6b5fcfb852e2d9c4/scripts/slave/recipe_modules/chromium_tests/steps.py
[modify] https://crrev.com/803f9d5158ec63ecbb8f73ec6b5fcfb852e2d9c4/scripts/slave/recipes/chromium_trybot.expected/swarmed_webkit_tests_unexpected_error.json
[modify] https://crrev.com/803f9d5158ec63ecbb8f73ec6b5fcfb852e2d9c4/scripts/slave/recipe_modules/chromium_tests/tests/api/run_tests_on_tryserver.expected/disable_deapply_patch_affected_files.json
[modify] https://crrev.com/803f9d5158ec63ecbb8f73ec6b5fcfb852e2d9c4/scripts/slave/recipe_modules/chromium_tests/tests/steps/local_gtest_test.expected/android.json
[modify] https://crrev.com/803f9d5158ec63ecbb8f73ec6b5fcfb852e2d9c4/scripts/slave/recipe_modules/chromium_tests/tests/steps/local_gtest_test.expected/retry.json
[modify] https://crrev.com/803f9d5158ec63ecbb8f73ec6b5fcfb852e2d9c4/scripts/slave/recipe_modules/chromium_tests/tests/api/run_tests_on_tryserver.expected/enable_retry_with_patch_recipes.json
[modify] https://crrev.com/803f9d5158ec63ecbb8f73ec6b5fcfb852e2d9c4/scripts/slave/recipes/chromium_trybot.expected/swarming_test_failure.json
[modify] https://crrev.com/803f9d5158ec63ecbb8f73ec6b5fcfb852e2d9c4/scripts/slave/recipes/chromium_trybot.expected/swarmed_layout_tests_too_many_failures_for_retcode.json
[modify] https://crrev.com/803f9d5158ec63ecbb8f73ec6b5fcfb852e2d9c4/scripts/slave/recipe_modules/chromium_tests/tests/steps/local_gtest_test.expected/basic.json
[modify] https://crrev.com/803f9d5158ec63ecbb8f73ec6b5fcfb852e2d9c4/scripts/slave/recipe_modules/chromium_tests/tests/api/run_tests_on_tryserver.expected/basic.json
[modify] https://crrev.com/803f9d5158ec63ecbb8f73ec6b5fcfb852e2d9c4/scripts/slave/recipe_modules/chromium_tests/tests/api/run_tests_on_tryserver.expected/nonzero_exit_code_no_gtest_output.json

Project Member

Comment 3 by bugdroid1@chromium.org, Nov 29

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

commit fc6b608eb369dc83788790ecdfeef238be27c985
Author: Takuto Ikuta <tikuta@chromium.org>
Date: Thu Nov 29 07:28:45 2018

Revert "'retry with patch' should only rerun failing tests 10 times."

This reverts commit 803f9d5158ec63ecbb8f73ec6b5fcfb852e2d9c4.

Reason for revert: broke fuchsia_x64 builder
https://bugs.chromium.org/p/chromium/issues/detail?id=910027

Original change's description:
> 'retry with patch' should only rerun failing tests 10 times.
> 
> 'retry with patch' uses --gtest_repeat=10 to rerun failing tests 10 times.
> Unfortunately, the default value of --test-launcher-retry-limit=4 causes
> deterministically failing tests to be rerun 40 times. This is not the intention.
> 
> This CL updates the recipe logic so that if we set --gtest_repeat=10, then we
> also set --test-launcher-retry-limit=1.
> 
> Bug: 909834
> Change-Id: I80b7972d7a08d161b8fa2a1f2c9e3f6c3af6a1a5
> Reviewed-on: https://chromium-review.googlesource.com/c/1351695
> Commit-Queue: Erik Chen <erikchen@chromium.org>
> Reviewed-by: John Budorick <jbudorick@chromium.org>

TBR=erikchen@chromium.org,jbudorick@chromium.org

Change-Id: I4182795c48a8913d3bdbb2c8a678f61d851ac7d6
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 909834
Reviewed-on: https://chromium-review.googlesource.com/c/1354141
Reviewed-by: Takuto Ikuta <tikuta@chromium.org>
Commit-Queue: Takuto Ikuta <tikuta@chromium.org>

[modify] https://crrev.com/fc6b608eb369dc83788790ecdfeef238be27c985/scripts/slave/recipes/chromium_trybot.expected/invalid_results.json
[modify] https://crrev.com/fc6b608eb369dc83788790ecdfeef238be27c985/scripts/slave/recipe_modules/chromium_tests/tests/api/run_tests_on_tryserver.expected/enable_retry_with_patch_succeed_after_deapply.json
[modify] https://crrev.com/fc6b608eb369dc83788790ecdfeef238be27c985/scripts/slave/recipe_modules/chromium_tests/tests/api/run_tests_on_tryserver.expected/enable_retry_with_patch_invalid_test_results.json
[modify] https://crrev.com/fc6b608eb369dc83788790ecdfeef238be27c985/scripts/slave/recipes/chromium_trybot.expected/swarmed_webkit_tests_interrupted.json
[modify] https://crrev.com/fc6b608eb369dc83788790ecdfeef238be27c985/scripts/slave/recipe_modules/chromium_tests/steps.py
[modify] https://crrev.com/fc6b608eb369dc83788790ecdfeef238be27c985/scripts/slave/recipes/chromium_trybot.expected/swarmed_webkit_tests_unexpected_error.json
[modify] https://crrev.com/fc6b608eb369dc83788790ecdfeef238be27c985/scripts/slave/recipe_modules/chromium_tests/tests/api/run_tests_on_tryserver.expected/disable_deapply_patch_affected_files.json
[modify] https://crrev.com/fc6b608eb369dc83788790ecdfeef238be27c985/scripts/slave/recipe_modules/chromium_tests/tests/steps/local_gtest_test.expected/android.json
[modify] https://crrev.com/fc6b608eb369dc83788790ecdfeef238be27c985/scripts/slave/recipe_modules/chromium_tests/tests/steps/local_gtest_test.expected/retry.json
[modify] https://crrev.com/fc6b608eb369dc83788790ecdfeef238be27c985/scripts/slave/recipe_modules/chromium_tests/tests/api/run_tests_on_tryserver.expected/enable_retry_with_patch_recipes.json
[modify] https://crrev.com/fc6b608eb369dc83788790ecdfeef238be27c985/scripts/slave/recipes/chromium_trybot.expected/swarming_test_failure.json
[modify] https://crrev.com/fc6b608eb369dc83788790ecdfeef238be27c985/scripts/slave/recipes/chromium_trybot.expected/swarmed_layout_tests_too_many_failures_for_retcode.json
[modify] https://crrev.com/fc6b608eb369dc83788790ecdfeef238be27c985/scripts/slave/recipe_modules/chromium_tests/tests/steps/local_gtest_test.expected/basic.json
[modify] https://crrev.com/fc6b608eb369dc83788790ecdfeef238be27c985/scripts/slave/recipe_modules/chromium_tests/tests/api/run_tests_on_tryserver.expected/basic.json
[modify] https://crrev.com/fc6b608eb369dc83788790ecdfeef238be27c985/scripts/slave/recipe_modules/chromium_tests/tests/api/run_tests_on_tryserver.expected/nonzero_exit_code_no_gtest_output.json

Project Member

Comment 4 by bugdroid1@chromium.org, Nov 29

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

commit f088edccfa5d2289368b10016884633fdaff610a
Author: erikchen <erikchen@chromium.org>
Date: Thu Nov 29 21:53:10 2018

[Reland 1] 'retry with patch' should only rerun failing tests 10 times.

The first attempt to land the CL was reverted as fuchsia/test_runner.py did not
recognize the --test-launcher-retry-limit argument. This has since been fixed.

Tested via LED at: https://chromium-swarm.appspot.com/task?id=4179ac0ead41db10

> 'retry with patch' uses --gtest_repeat=10 to rerun failing tests 10 times.
> Unfortunately, the default value of --test-launcher-retry-limit=4 causes
> deterministically failing tests to be rerun 40 times. This is not the intention.
>
> This CL updates the recipe logic so that if we set --gtest_repeat=10, then we
> also set --test-launcher-retry-limit=1.
>
> Bug: 909834
> Change-Id: I80b7972d7a08d161b8fa2a1f2c9e3f6c3af6a1a5
> Reviewed-on: https://chromium-review.googlesource.com/c/1351695
> Commit-Queue: Erik Chen <erikchen@chromium.org>
> Reviewed-by: John Budorick <jbudorick@chromium.org>

Bug: 909834
Change-Id: I5105e847edf272e51bbc6fbb734f6897793d55e3
Reviewed-on: https://chromium-review.googlesource.com/c/1351700
Reviewed-by: John Budorick <jbudorick@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>

[modify] https://crrev.com/f088edccfa5d2289368b10016884633fdaff610a/scripts/slave/recipes/chromium_trybot.expected/invalid_results.json
[modify] https://crrev.com/f088edccfa5d2289368b10016884633fdaff610a/scripts/slave/recipe_modules/chromium_tests/tests/api/run_tests_on_tryserver.expected/enable_retry_with_patch_succeed_after_deapply.json
[modify] https://crrev.com/f088edccfa5d2289368b10016884633fdaff610a/scripts/slave/recipe_modules/chromium_tests/tests/api/run_tests_on_tryserver.expected/enable_retry_with_patch_invalid_test_results.json
[modify] https://crrev.com/f088edccfa5d2289368b10016884633fdaff610a/scripts/slave/recipes/chromium_trybot.expected/swarmed_webkit_tests_interrupted.json
[modify] https://crrev.com/f088edccfa5d2289368b10016884633fdaff610a/scripts/slave/recipe_modules/chromium_tests/steps.py
[modify] https://crrev.com/f088edccfa5d2289368b10016884633fdaff610a/scripts/slave/recipes/chromium_trybot.expected/swarmed_webkit_tests_unexpected_error.json
[modify] https://crrev.com/f088edccfa5d2289368b10016884633fdaff610a/scripts/slave/recipe_modules/chromium_tests/tests/api/run_tests_on_tryserver.expected/disable_deapply_patch_affected_files.json
[modify] https://crrev.com/f088edccfa5d2289368b10016884633fdaff610a/scripts/slave/recipe_modules/chromium_tests/tests/steps/local_gtest_test.expected/android.json
[modify] https://crrev.com/f088edccfa5d2289368b10016884633fdaff610a/scripts/slave/recipe_modules/chromium_tests/tests/steps/local_gtest_test.expected/retry.json
[modify] https://crrev.com/f088edccfa5d2289368b10016884633fdaff610a/scripts/slave/recipe_modules/chromium_tests/tests/api/run_tests_on_tryserver.expected/enable_retry_with_patch_recipes.json
[modify] https://crrev.com/f088edccfa5d2289368b10016884633fdaff610a/scripts/slave/recipes/chromium_trybot.expected/swarming_test_failure.json
[modify] https://crrev.com/f088edccfa5d2289368b10016884633fdaff610a/scripts/slave/recipes/chromium_trybot.expected/swarmed_layout_tests_too_many_failures_for_retcode.json
[modify] https://crrev.com/f088edccfa5d2289368b10016884633fdaff610a/scripts/slave/recipe_modules/chromium_tests/tests/steps/local_gtest_test.expected/basic.json
[modify] https://crrev.com/f088edccfa5d2289368b10016884633fdaff610a/scripts/slave/recipe_modules/chromium_tests/tests/api/run_tests_on_tryserver.expected/basic.json
[modify] https://crrev.com/f088edccfa5d2289368b10016884633fdaff610a/scripts/slave/recipe_modules/chromium_tests/tests/api/run_tests_on_tryserver.expected/nonzero_exit_code_no_gtest_output.json

Sign in to add a comment