New issue
Advanced search Search tips

Issue 677895 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Closed: Jan 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

factory: start test should not expect EC RO=RW when ectool is available.

Project Member Reported by hungte@chromium.org, Jan 3 2017

Issue description

When dargs.has_ectool is True, pytests/start will try to read RO/RW and fail if they don't match. This was introduced since https://code.google.com/p/chrome-os-partner/issues/detail?id=12615#c2

However, there are few problems:
 1. The arg name 'has_ectool' is not clear enough for its behavior (checking ro==rw).
 2. Since EC software sync was introduced, there are projects that do need to have
    different EC RO/RW.

I think we should rename this to a new var like "check_ec_has_same_ro_rw".
 
Owner: hungte@chromium.org
Status: Started (was: Untriaged)
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/factory/+/9b7536b41b851927dc90681df57c391d8a81edd0

commit 9b7536b41b851927dc90681df57c391d8a81edd0
Author: Hung-Te Lin <hungte@chromium.org>
Date: Wed Jan 04 06:18:43 2017

pytests: Drop has_ectool and EC RO/RW version checks in start.

Checking EC RO=RW is not what a "start" test should do, because:
 - There are projects that do need RO != RW. Having has_ectool does not
   mean they want to check EC RO/RW versions.
 - Start is a general test that should not check such details.

If any project that needs to ensure EC RO=RW, they should add a
probe or verify_value or private test in test list, immediately after
start.

BUG= chromium:677895 
TEST=None
CQ-DEPEND=CL:*315275

Change-Id: Id979fcf27ee5fd590391c326bb27c5b013ab1df9
Reviewed-on: https://chromium-review.googlesource.com/424461
Commit-Ready: Hung-Te Lin <hungte@chromium.org>
Tested-by: Hung-Te Lin <hungte@chromium.org>
Reviewed-by: Wei-Han Chen <stimim@chromium.org>

[modify] https://crrev.com/9b7536b41b851927dc90681df57c391d8a81edd0/py/test/pytests/start/start.py

Status: Fixed (was: Started)
Project Member

Comment 4 by bugdroid1@chromium.org, Feb 3 2017

Labels: merge-merged-factory-veyron-6591.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/factory/+/01e0e04b3f0ec2baf14165db948fabaf1d9a1d5d

commit 01e0e04b3f0ec2baf14165db948fabaf1d9a1d5d
Author: Hung-Te Lin <hungte@chromium.org>
Date: Fri Feb 03 23:13:49 2017

Cherry-pick: pytests: Drop has_ectool and EC RO/RW version checks in start.

Checking EC RO=RW is not what a "start" test should do, because:
 - There are projects that do need RO != RW. Having has_ectool does not
   mean they want to check EC RO/RW versions.
 - Start is a general test that should not check such details.

If any project that needs to ensure EC RO=RW, they should add a
probe or verify_value or private test in test list, immediately after
start.

BUG= chromium:677895 
TEST=None
CQ-DEPEND=CL:*315275

Original Change-Id: Id979fcf27ee5fd590391c326bb27c5b013ab1df9
Original Reviewed-on: https://chromium-review.googlesource.com/424461
Original Commit-Ready: Hung-Te Lin <hungte@chromium.org>
Original Tested-by: Hung-Te Lin <hungte@chromium.org>
Original Reviewed-by: Wei-Han Chen <stimim@chromium.org>
(cherry picked from commit 9b7536b41b851927dc90681df57c391d8a81edd0)

Conflicts:
	py/test/pytests/start/start.py

Change-Id: I3c13e4f383c65b2d9a11364c9e7ee0e485f632f1
Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
Reviewed-on: https://chromium-review.googlesource.com/426283
Tested-by: Vineeth Starly <vineeths@google.com>
Reviewed-by: Philip Chen <philipchen@chromium.org>

[modify] https://crrev.com/01e0e04b3f0ec2baf14165db948fabaf1d9a1d5d/py/test/pytests/start/start.py

Comment 5 by dchan@google.com, Mar 4 2017

Labels: VerifyIn-58
Project Member

Comment 6 by bugdroid1@chromium.org, Mar 6 2017

Project Member

Comment 7 by bugdroid1@chromium.org, Mar 22 2017

Project Member

Comment 8 by bugdroid1@chromium.org, Mar 29 2017

Comment 9 by dchan@google.com, Apr 17 2017

Labels: VerifyIn-59

Comment 10 by dchan@google.com, May 30 2017

Labels: VerifyIn-60
Labels: VerifyIn-61

Comment 12 by dchan@chromium.org, Oct 14 2017

Status: Archived (was: Fixed)

Sign in to add a comment