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

Issue 760336 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Last visit > 30 days ago
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

factory: cleanup: pytests should prevent calling shopfloor (factory_server) directly.

Project Member Reported by hungte@chromium.org, Aug 29 2017

Issue description

Currently many station tests hard coded same logic in their test implementation:
pytests/bluetooth.py
pytests/rf_graphyte/rf_graphyte.py
pytests/vswr/vswr.py
pytests/audio_quality.py
pytests/retrieve_config.py
pytests/als_fixture.py
pytests/camera_fixture.py
pytests/touchscreen_calibration/touchscreen_calibration.py

They all (1) call GetParameter if a param is set (2) call SaveAuxLog to upload something.

By our design today, GetParameter should be done by adding a `retrieve_config` test into test list, and SaveAuxLog should be replaced by testlog.AttachFile.

(Note retrieve_config currently only reads JSON config, but some tests also download resource files like *.zip plus md5 - so we may need to either use cros_payload for them, or extend retrieve_config to take more files).
 

Comment 1 by hungte@chromium.org, Aug 29 2017

Owner: chuntsen@chromium.org
Status: Assigned (was: Untriaged)
@chuntsen, can you start by changing all the SaveAuxLog and shopfloor.UploadAuxLogs to testlog.AttachFile?
Do we need to upload all logs without config?
Or have a "upload_log" argument and default True/False?
I think previously they make it optional because UploadAuxLogs takes extra time and will fail if sync can't be done.

So for us, attach / log directly is fine, since it's instalog nodes to decide if it should keep logs.

You can still have arguments - but it should not be "upload_log" since we don't upload in the pytest. It's sent to testlog. Maybe change the param to something more close to what it's logged, for example "log_report".
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 25 2017

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

commit da6df341912eb2afda062e77b87596bef118c16e
Author: Wei-Han Chen <stimim@google.com>
Date: Wed Oct 25 12:52:44 2017

audio_quality: always upload log files

We used to only upload log files to factory server when
``enable_factory_server`` is true, because the upload API requires
internet connection while testing.  Now we are using testlog and
instalog, which allow us to add files to logs and upload them in the
background.  So we can always upload log files no matter
``enable_factory_server`` is true or not.  Now ``enable_factory_server``
only affects downloading parameters from factory server or not.

BUG=chromium:760336
TEST=None

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

[modify] https://crrev.com/da6df341912eb2afda062e77b87596bef118c16e/po/zh-CN.po
[modify] https://crrev.com/da6df341912eb2afda062e77b87596bef118c16e/py/test/pytests/audio_quality.py

The only one left is offline_test, which I think it can still be solved by Attach. Do you agree, stimim?

Sign in to add a comment