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

Issue 873716 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 23
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: ----



Sign in to add a comment

Skylab: Drop support for RETRIES from Autotest

Project Member Reported by xixuan@chromium.org, Aug 13

Issue description

dummy_Fail.RetrySuccess is one test in client/site_tests/dummy_Fail/control.retry_alwaysflake

https://chromium-swarm-dev.appspot.com/task?id=3f4cd7e498184e10&refresh=10

It reports all of the 2 dummy_Fail tests as failure:
RetryFail
RetrySuccess
 
Status: Started (was: Untriaged)
Well, dummy_Fail test is supposed to fail...
So, as far as I can tell, RetrySuccess is supposed to fail too.  It's set up to pass after running dummy_Fail 5 times.  But control.retry_alwaysflake only runs dummy_Fail 2 times:

RetryFail
RetrySuccess

If I changed the test to run

RetryFail
RetryFail
RetryFail
RetryFail
RetrySuccess

then it should succeed.
Oh, it's supposed to run with test_retry = 5 or it won't pass
Lucifer has a -x-test-retries flag, so it just needs to be passed in from... somewhere
Well, it comes from the Job AFE model in Autotest, and the default value for that comes from RETRIES in the control file, yuck
Cc: -pprabhu@chromium.org akes...@chromium.org ayatane@chromium.org
Owner: pprabhu@chromium.org
The (almost made) decision here is to kill the RETRIES feature.

Note that there are two separate notions of retries, that can both be controlled via control file constants.

- RETRIES (this one): A retry within autoserv. skylab/afe does not know that the test was retried, there aren't separate TKO entries for the retried test. i.e., we're blind that the retry happened.
- JOB_RETRIES: Suite level option -- test is retried (possibly on a different host). Suite has other related options that controls how many tests can be retried across the suite etc.

We will kill RETRIES because:
- No that many things use it: http://shortn/_KTM7dKkHdr
- We're blind when this happens / how often this happens.
- Often when a test fails, we want it to run on a different DUT.
- It is redundant with JOB_RETRIES
- It is non-trivial to make it work with Skylab.

There is a possible performance benefit of RETRIES over JOB_RETRIES, but such retries should be done at an even tighter scope -- the test started a little webserver on the DUT that failed to come up? Well, retry starting the webserver, don't ask autoserv to retry the test for you.

So, I'm going to go ahead and cull this ancient feature.
This is also not specified in that many control files: http://shortn/_9OWKA1F2nm
All tests that use this feature will be bumped to instead use JOB_RETRIES.

Comment 8 Deleted

Cc: haddowk@chromium.org
https://chromium-review.googlesource.com/c/chromiumos/third_party/autotest/+/1173661 

This stack drops the feature by not respecting RETRIES anymore within autoserv, and migrates the smattering of users to JOB_RETRIES.

Cleanup is to remove the test.retries argument from all over the place.

+haddowk to make sure nobody uses this on moblab either.
Summary: Skylab: Drop support for RETRIES from Autotest (was: skylab test push: dummy_Fail.RetrySuccess Expected: [GOOD], Actual: [FAIL])
Cc: mattmallett@chromium.org
from a cts point of view it is ok +mattmallet to check for the device qualification tests.
Not needed for component qualifications. OK to drop
Project Member

Comment 13 by bugdroid1@chromium.org, Aug 16

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/6e24a9b161943a9cc226d1cde9e99671ad84a9bb

commit 6e24a9b161943a9cc226d1cde9e99671ad84a9bb
Author: Prathmesh Prabhu <pprabhu@chromium.org>
Date: Thu Aug 16 20:14:33 2018

autotest: Drop RETRIES related tests from test_push

The RETRIES features is going to be deleted, so stop promising that it
will work.

BUG= chromium:873716 
TEST=None

Change-Id: Ia0a70ae784e991ca57b4b6f2e1baf5a44ea3a7df
Reviewed-on: https://chromium-review.googlesource.com/1173147
Commit-Ready: Prathmesh Prabhu <pprabhu@chromium.org>
Tested-by: Prathmesh Prabhu <pprabhu@chromium.org>
Reviewed-by: Prathmesh Prabhu <pprabhu@chromium.org>

[delete] https://crrev.com/88fac1ff5e22958f63722d80fe8114f9722bd304/client/tests/flaky_test/flaky_test.py
[delete] https://crrev.com/88fac1ff5e22958f63722d80fe8114f9722bd304/client/site_tests/dummy_Fail/control.retry_alwaysfail
[delete] https://crrev.com/88fac1ff5e22958f63722d80fe8114f9722bd304/client/tests/flaky_test/control
[delete] https://crrev.com/88fac1ff5e22958f63722d80fe8114f9722bd304/server/site_tests/dummy_FlakyTestServer/flaky_test.py
[delete] https://crrev.com/88fac1ff5e22958f63722d80fe8114f9722bd304/client/site_tests/dummy_Fail/control.retry_alwaysflake
[delete] https://crrev.com/88fac1ff5e22958f63722d80fe8114f9722bd304/server/site_tests/dummy_FlakyTestServer/control
[delete] https://crrev.com/88fac1ff5e22958f63722d80fe8114f9722bd304/server/site_tests/dummy_FailServer/control.retry_failfast
[modify] https://crrev.com/6e24a9b161943a9cc226d1cde9e99671ad84a9bb/site_utils/attribute_whitelist.txt
[modify] https://crrev.com/6e24a9b161943a9cc226d1cde9e99671ad84a9bb/site_utils/test_push.py
[delete] https://crrev.com/88fac1ff5e22958f63722d80fe8114f9722bd304/client/tests/flaky_test/control.noretry

Project Member

Comment 14 by bugdroid1@chromium.org, Aug 16

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/ee32addc6b16dc9b88e655320e38f78ead2cc8cb

commit ee32addc6b16dc9b88e655320e38f78ead2cc8cb
Author: Prathmesh Prabhu <pprabhu@chromium.org>
Date: Thu Aug 16 20:14:34 2018

Migrate tests from RETRIES to JOB_RETRIES

RETRIES is an old control file option that triggers retries within
autoserv that are mostly invisible to the rest of the system. This
feature is being killed.
Instead, use JOB_RETRIES that directs the suite that kicks off tests to
retry the test within sane limits.

BUG= chromium:873716 
TEST=None

Change-Id: I72c184e8c33cd23367192df7b7bbfdcc16983414
Reviewed-on: https://chromium-review.googlesource.com/1173655
Commit-Ready: Prathmesh Prabhu <pprabhu@chromium.org>
Tested-by: Prathmesh Prabhu <pprabhu@chromium.org>
Reviewed-by: Prathmesh Prabhu <pprabhu@chromium.org>
Reviewed-by: Allen Li <ayatane@chromium.org>

[modify] https://crrev.com/ee32addc6b16dc9b88e655320e38f78ead2cc8cb/client/site_tests/video_JpegDecodeAccelerator/control
[modify] https://crrev.com/ee32addc6b16dc9b88e655320e38f78ead2cc8cb/client/site_tests/cellular_ModemControl/control.novatel-e362
[modify] https://crrev.com/ee32addc6b16dc9b88e655320e38f78ead2cc8cb/client/site_tests/webrtc_PausePlayPeerConnections/control.audio
[modify] https://crrev.com/ee32addc6b16dc9b88e655320e38f78ead2cc8cb/client/site_tests/video_VideoDecodeAccelerator/control.h264
[modify] https://crrev.com/ee32addc6b16dc9b88e655320e38f78ead2cc8cb/client/site_tests/webrtc_PausePlayPeerConnections/control.video
[modify] https://crrev.com/ee32addc6b16dc9b88e655320e38f78ead2cc8cb/client/site_tests/video_VideoDecodeAccelerator/control.vp8
[modify] https://crrev.com/ee32addc6b16dc9b88e655320e38f78ead2cc8cb/client/site_tests/video_VideoDecodeAccelerator/control.vp9
[modify] https://crrev.com/ee32addc6b16dc9b88e655320e38f78ead2cc8cb/client/site_tests/video_JpegEncodeAccelerator/control
[modify] https://crrev.com/ee32addc6b16dc9b88e655320e38f78ead2cc8cb/client/site_tests/video_WebRtcMainFeedSwitching/control
[modify] https://crrev.com/ee32addc6b16dc9b88e655320e38f78ead2cc8cb/client/site_tests/video_VideoDecodeAccelerator/control.vp9_2
[modify] https://crrev.com/ee32addc6b16dc9b88e655320e38f78ead2cc8cb/client/site_tests/webrtc_PausePlayPeerConnections/control.audio_perf
[modify] https://crrev.com/ee32addc6b16dc9b88e655320e38f78ead2cc8cb/client/site_tests/telemetry_AFDOGenerateClient/control
[modify] https://crrev.com/ee32addc6b16dc9b88e655320e38f78ead2cc8cb/client/site_tests/webrtc_PausePlayPeerConnections/control.video_perf
[modify] https://crrev.com/ee32addc6b16dc9b88e655320e38f78ead2cc8cb/client/site_tests/video_WebRtcResolutionSwitching/control
[modify] https://crrev.com/ee32addc6b16dc9b88e655320e38f78ead2cc8cb/client/site_tests/video_WebRtcMainFeedSwitching/control.perf
[modify] https://crrev.com/ee32addc6b16dc9b88e655320e38f78ead2cc8cb/client/site_tests/video_WebRtcResolutionSwitching/control.perf

Project Member

Comment 15 by bugdroid1@chromium.org, Aug 16

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/b853d5d59b44350293f351a489325834eeb87bda

commit b853d5d59b44350293f351a489325834eeb87bda
Author: Prathmesh Prabhu <pprabhu@chromium.org>
Date: Thu Aug 16 20:14:35 2018

autotest: Drop error.TestFailRetry

This is the first CL in dropping support for the RETRIES directive.
This CL removes the special exception that caused autoserv to retry
tests internally.

BUG= chromium:873716 
TEST=None

Change-Id: I87e7272d443bb3f8144424af3c673b694b873923
Reviewed-on: https://chromium-review.googlesource.com/1173661
Commit-Ready: Prathmesh Prabhu <pprabhu@chromium.org>
Tested-by: Prathmesh Prabhu <pprabhu@chromium.org>
Reviewed-by: Prathmesh Prabhu <pprabhu@chromium.org>
Reviewed-by: Allen Li <ayatane@chromium.org>

[modify] https://crrev.com/b853d5d59b44350293f351a489325834eeb87bda/client/common_lib/test_unittest.py
[modify] https://crrev.com/b853d5d59b44350293f351a489325834eeb87bda/client/common_lib/test.py
[modify] https://crrev.com/b853d5d59b44350293f351a489325834eeb87bda/client/common_lib/error.py
[modify] https://crrev.com/b853d5d59b44350293f351a489325834eeb87bda/client/site_tests/platform_BootLockbox/platform_BootLockbox.py
[modify] https://crrev.com/b853d5d59b44350293f351a489325834eeb87bda/client/site_tests/telemetry_AFDOGenerateClient/telemetry_AFDOGenerateClient.py
[modify] https://crrev.com/b853d5d59b44350293f351a489325834eeb87bda/client/common_lib/cros/test_webrtc_peer_connection.py

test_push is failing. It is still expected to run "dummy_Fail.RetrySuccess" as they use the old package like gandof-release/R64-10176.65.0.


__main__.TestPushException: 1 test(s) are not expected to be run:
dummy_Fail.RetrySuccess
I'll take a look.
Project Member

Comment 19 by bugdroid1@chromium.org, Aug 17

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/c4577987380be77578e00d03711e74c11395380b

commit c4577987380be77578e00d03711e74c11395380b
Author: Prathmesh Prabhu <pprabhu@chromium.org>
Date: Fri Aug 17 21:46:06 2018

test_push: Ignore mismatched results due to old target build

BUG= chromium:873716 
TEST=test_push passes

Change-Id: I17f49839a3f166aaa49fb0bae5e579b6c3ebdce2
Reviewed-on: https://chromium-review.googlesource.com/1180163
Reviewed-by: Xixuan Wu <xixuan@chromium.org>
Commit-Queue: Xixuan Wu <xixuan@chromium.org>
Tested-by: Xixuan Wu <xixuan@chromium.org>

[modify] https://crrev.com/c4577987380be77578e00d03711e74c11395380b/site_utils/test_push.py

Project Member

Comment 20 by bugdroid1@chromium.org, Aug 23

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/bcc5b7ecd243b1ebcbca331b7d3357f51382fdf4

commit bcc5b7ecd243b1ebcbca331b7d3357f51382fdf4
Author: Prathmesh Prabhu <pprabhu@chromium.org>
Date: Thu Aug 23 14:36:41 2018

autotest: Delete autoserv_utils._autoserv_command_line()

This was an unnecessary private function being called from outside the
module.

BUG= chromium:873716 
TEST=None

Change-Id: Ic701a5c4372dd25a71fcc3697bcdeeefdb34b71f
Reviewed-on: https://chromium-review.googlesource.com/1180562
Commit-Ready: Prathmesh Prabhu <pprabhu@chromium.org>
Tested-by: Prathmesh Prabhu <pprabhu@chromium.org>
Reviewed-by: Allen Li <ayatane@chromium.org>

[modify] https://crrev.com/bcc5b7ecd243b1ebcbca331b7d3357f51382fdf4/scheduler/agent_task.py
[modify] https://crrev.com/bcc5b7ecd243b1ebcbca331b7d3357f51382fdf4/scheduler/prejob_task.py
[modify] https://crrev.com/bcc5b7ecd243b1ebcbca331b7d3357f51382fdf4/server/autoserv_utils.py

Project Member

Comment 21 by bugdroid1@chromium.org, Aug 23

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/ce20b33ca9d9b055fd087cdcbe9f3f5b4416f1e9

commit ce20b33ca9d9b055fd087cdcbe9f3f5b4416f1e9
Author: Prathmesh Prabhu <pprabhu@chromium.org>
Date: Thu Aug 23 14:36:41 2018

autotest: Stop passing --test-retries to autoserv

BUG= chromium:873716 
TEST=None

Change-Id: I72c40495e7822ed220429fc9a029c3ff85e5ee19
Reviewed-on: https://chromium-review.googlesource.com/1180563
Commit-Ready: Prathmesh Prabhu <pprabhu@chromium.org>
Tested-by: Prathmesh Prabhu <pprabhu@chromium.org>
Reviewed-by: Allen Li <ayatane@chromium.org>

[modify] https://crrev.com/ce20b33ca9d9b055fd087cdcbe9f3f5b4416f1e9/venv/lucifer/cmd/job_reporter.py
[modify] https://crrev.com/ce20b33ca9d9b055fd087cdcbe9f3f5b4416f1e9/scheduler/monitor_db.py
[modify] https://crrev.com/ce20b33ca9d9b055fd087cdcbe9f3f5b4416f1e9/server/autoserv_utils.py

Project Member

Comment 22 by bugdroid1@chromium.org, Aug 23

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/7db71a332d538fad8210d570d5accda89c335b0a

commit 7db71a332d538fad8210d570d5accda89c335b0a
Author: Prathmesh Prabhu <pprabhu@chromium.org>
Date: Thu Aug 23 14:36:42 2018

autotest: Drop --test-retry argument from autoserv

BUG= chromium:873716 
TEST=None

Change-Id: Iea4a9f25e5a2abdf48627da11af0938a588782b7
Reviewed-on: https://chromium-review.googlesource.com/1180564
Commit-Ready: Prathmesh Prabhu <pprabhu@chromium.org>
Tested-by: Prathmesh Prabhu <pprabhu@chromium.org>
Reviewed-by: Allen Li <ayatane@chromium.org>

[modify] https://crrev.com/7db71a332d538fad8210d570d5accda89c335b0a/server/autoserv_parser.py
[modify] https://crrev.com/7db71a332d538fad8210d570d5accda89c335b0a/server/autoserv

Project Member

Comment 23 by bugdroid1@chromium.org, Aug 23

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/15ec22e36166e6fdf3de33c864510cad5c2cd36d

commit 15ec22e36166e6fdf3de33c864510cad5c2cd36d
Author: Prathmesh Prabhu <pprabhu@chromium.org>
Date: Thu Aug 23 14:36:43 2018

autotest: Pin test_retry to 0 in AFE RPCs

test_retry is deprecated. This CL pins the value of this parameter to 0
in the job creation RPCs. The argument is not yet removed entirely from
the models.

BUG= chromium:873716 
TEST=None

Change-Id: I31269389056f02d8e64a72aeee1771a414e0247f
Reviewed-on: https://chromium-review.googlesource.com/1180565
Commit-Ready: Prathmesh Prabhu <pprabhu@chromium.org>
Tested-by: Prathmesh Prabhu <pprabhu@chromium.org>
Reviewed-by: Allen Li <ayatane@chromium.org>

[modify] https://crrev.com/15ec22e36166e6fdf3de33c864510cad5c2cd36d/frontend/afe/rpc_utils.py
[modify] https://crrev.com/15ec22e36166e6fdf3de33c864510cad5c2cd36d/frontend/afe/rpc_interface_unittest.py
[modify] https://crrev.com/15ec22e36166e6fdf3de33c864510cad5c2cd36d/frontend/afe/rpc_interface.py

Project Member

Comment 24 by bugdroid1@chromium.org, Aug 23

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/7d08ac11c9212ef1f8d56457ca5fa87afcd32cdd

commit 7d08ac11c9212ef1f8d56457ca5fa87afcd32cdd
Author: Prathmesh Prabhu <pprabhu@chromium.org>
Date: Thu Aug 23 14:36:43 2018

autotest: Drop test_retry from autotest job objects

BUG= chromium:873716 
TEST=None

Change-Id: I7754c20b051d3fb1115d5e3bd237cbc6156c2e49
Reviewed-on: https://chromium-review.googlesource.com/1180566
Commit-Ready: Prathmesh Prabhu <pprabhu@chromium.org>
Tested-by: Prathmesh Prabhu <pprabhu@chromium.org>
Reviewed-by: Allen Li <ayatane@chromium.org>

[modify] https://crrev.com/7d08ac11c9212ef1f8d56457ca5fa87afcd32cdd/server/server_job.py
[modify] https://crrev.com/7d08ac11c9212ef1f8d56457ca5fa87afcd32cdd/client/common_lib/base_job.py
[modify] https://crrev.com/7d08ac11c9212ef1f8d56457ca5fa87afcd32cdd/client/common_lib/base_job_unittest.py

Project Member

Comment 25 by bugdroid1@chromium.org, Aug 23

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/94df5549c56fb17de484870ed41a1015b6e67e97

commit 94df5549c56fb17de484870ed41a1015b6e67e97
Author: Prathmesh Prabhu <pprabhu@chromium.org>
Date: Thu Aug 23 14:36:44 2018

autotest: Stop creating job with test_retries set in dynamic_suite

BUG= chromium:873716 
TEST=None

Change-Id: Ic9210af88bc9e948d69eb3aae99f271ef6a2349b
Reviewed-on: https://chromium-review.googlesource.com/1180567
Commit-Ready: Prathmesh Prabhu <pprabhu@chromium.org>
Tested-by: Prathmesh Prabhu <pprabhu@chromium.org>
Reviewed-by: Allen Li <ayatane@chromium.org>

[modify] https://crrev.com/94df5549c56fb17de484870ed41a1015b6e67e97/server/cros/dynamic_suite/suite.py
[modify] https://crrev.com/94df5549c56fb17de484870ed41a1015b6e67e97/server/cros/dynamic_suite/suite_unittest.py

Status: Fixed (was: Started)
All done here. The feature is dead (sans some scheduler DB models that are not worth the time to cull)
Project Member

Comment 28 by bugdroid1@chromium.org, Aug 29

Sign in to add a comment