factory: Clean unused, obsoleted and non-tests in py/test/pytests |
|||||||
Issue descriptionCurrently when you do "make doc", there will be few warnings: WARNING:root:No test cases found in cros.factory.test.pytests.tablet_mode_ui WARNING:root:No test cases found in cros.factory.test.pytests.wifi_throughput_host wifi_throuput_host is a script to run on host side, and is not a real pytest. I think it should live in somewhere else, for example py/test/rf or py/tools, or move wifi_throuput to its own folder, and put wifi_throuput_host there - but I need RF experts to confirm how this file was deployed (kitching/akahuang/petershih?) tablet_mode_ui is actually a shared UI template for few tablet tests (tablet_mode, tablet_rotation) to share. I think we should either move all tablet tests to a folder (tablet/) so test items can share files there, or move tablet_mode_ui to a new ui template in py/test/, for example py/test/tablet_mode_ui or merged into ui_templates.
,
Jul 26 2017
In addition to non-test, I think maybe it's time to remove the unused/obsolete RF pytest. List of RF pytests that are active in core project: - bluetooth_host: station-based bluetooth scan/pairing test. - rf_graphyte: conductive/radiated test using Graphyte. - vswr: VSWR test, need external instrument. - wifi_throughput: station-based Wifi throughput test. Need external AP List of RF pytests that are active in partner project: - wireless_antenna - wireless_radiotap The list of pytests that I'm not sure if we will use in the future: - cellular_switch_firmware - probe_cellular_info - lte_verify_config - bluetooth: DUT-based bluetooth test. Need remote bluetooth device (e.g keyboard) The list I think/guess obsolete: - rf_radiated - wifi_rf - wifi_throughput_host
,
Jul 26 2017
,
Jul 26 2017
,
Jul 26 2017
wireless_antenna and wireless_radiotap should be merged (issue:748475). cellular_switch_firmware will need to be reviewed. probe_cellular_info should be replaced by probe if possible, and lte_verify_config may be replaced by probe or verify_value, and I think bluetooth is still needed (I've seen partners testing this with external BT mouse). Agree rf_radiated is deprecated by rf_graphyte. Which test has deprecated wifi_rf? Still rf_graphyte?
,
Jul 26 2017
sensor_movement can be removed. It is designed for Lucid to test gyro, and we didn't have a gyro test at that time. Should be deprecated by gyroscope and compass test. hwmon_write is for Lucid too, just for simplify shell script logic in line_check_item. Not sure if we still need this.
,
Jul 26 2017
line_check_item is largely simplified by exec_shell. I'd prefer to remove hwmon_write if possible.
,
Jul 26 2017
p.s. does that mean we can also remove hwmon from Device API? What about py/test/pytests/hwmon_probe? (That should be merged to probe as well I think)
,
Jul 26 2017
dut.hwmon is used to read humidity/voltage/temperature sensors in Lucid, it's better to create components for each of them if needed, not using a generic hwmon component. hwmon_probe is more similar to thermal_sensor test, we can implement a hwmon thermal source and use thermal_sensor instead.
,
Jul 26 2017
> hwmon_probe is more similar to thermal_sensor test, we can implement a hwmon thermal source and use thermal_sensor instead. Agree. Meanwhile I've checked current test lists on ToT and no one is using that, so I think we can remove it until it's really needed by any real projects. > dut.hwmon is used to read humidity/voltage/temperature sensors in Lucid, it's better to create components for each of them if needed, not using a generic hwmon component. Since hwmon is actually not a real 'hardware component", I think maybe a better way is to move hwmon to cros.factory.test.utils.hwmon_utils, (or device.hwmon_utils, just like sensor_utils) and let individual sensors include hwmon.
,
Jul 26 2017
I'm wondering if rf_radiated can be replaced by rf_graphyte under all circumstances? rf_radiated measures signals via a power meter, I think this cannot be done by the graphyte framework? If this is the case, should we deprecate rf_radiated?
,
Jul 26 2017
Can we extend Graphyte to support measuring signals via power meter?
,
Jul 26 2017
Re #5, wifi_rf mainly check if DUT can connect to Wifi AP and measure the signal strength. It can be replaced by wifi_throughput. Re #12, The current radiated test with Graphyte framework also measure the power level of the signal by the power meter. In addition, we can support multiple brand of power meters (we support Anritsu ML2437a and Keysight N1914a now). rf_radiated pytest only supports Keysight N1914a power meter. So it's fine to deprecate rf_radiated pytest.
,
Jul 26 2017
Re #14, Thanks for the clarify. Then I guess we can deprecate rf_radiated, and use graphyte for future builds.
,
Jul 26 2017
also please set -x simple_storage_stress_test.py
,
Jul 26 2017
More recommendations: - remove ectool_i2c_dev_id.py (inactive test from all tot test lists) - rename ec_lightbar. It should be named lightbar if we really want, but since no OEM projects will be using it and it's very simple, highly depends on particular board, we should have it in board overlay if needed. - I wonder if we should rename als_fixture and light_sensor in same style. Maybe als and als_fixture, or light_sensor and light_sensor_calibration.
,
Jul 26 2017
... and maybe we should rename *_battery to battery_*. i.e., change simple_battey to battery_simple, and sysfs_battery to battery_sysfs.
,
Jul 27 2017
Some conclusions from above discussions. Following pytests will be removed: 1. [#2,#14] Remove rf_radiated, wifi_rf, wifi_throughput_host 2. [#5] Merge wireless_antenna and wireless_radiotap 3. [#6] Remove sensor_movement 4. [#6,#7] Remove hwmon_write 5. [#11] Remove hwmon_probe 6. [#17] Remove ectool_i2c_dev_id.py Following pytests might need to be renamed: 1. [#17] als_fixture 2. [#17] light_sensor 3. [#18] *_battery to battery_* Misc actions: 1. [#16] set -x simple_storage_stress_test.py Question 1: Comments #9, #10, #11 are around hwmon, thermal_sensor, and Device API. Should we create another issue on these topics? Question 2: From #17, if ec_lightbar should be in board overlay, I think we should remove it from ToT now?
,
Jul 27 2017
Q1. thermal sensor is already addressed in https://chromium-review.googlesource.com/#/c/586215/ but yes, you can create another CL for hwmon changes. Q2. yes I think we should remove it from ToT.
,
Jul 27 2017
#17 you can move that test to samus' board overlay factory-board, since that should be the only device using it with tot factory-board (link also has lightbar but its tests were incomplete so no factory-board).
,
Jul 27 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/factory/+/f1d20dcd86cb029b7b40a5f9115621302542017f commit f1d20dcd86cb029b7b40a5f9115621302542017f Author: Shen-En Shih <petershih@chromium.org> Date: Thu Jul 27 09:42:53 2017 simple_storage_stress_test: remove execution bit Remove the execution bit of pytest 'simple_storage_stress_test' BUG= chromium:748540 TEST=make test Change-Id: Id513d006babc9ce9129361f2751383b13088b7c9 Reviewed-on: https://chromium-review.googlesource.com/588527 Commit-Ready: Shen-En Shih <petershih@chromium.org> Tested-by: Shen-En Shih <petershih@chromium.org> Reviewed-by: Hung-Te Lin <hungte@chromium.org> [modify] https://crrev.com/f1d20dcd86cb029b7b40a5f9115621302542017f/py/test/pytests/simple_storage_stress_test.py
,
Jul 27 2017
The following revision refers to this bug: https://chrome-internal.googlesource.com/chromeos/overlays/overlay-samus-private/+/3ca3e6e16df706867db79f92e81246c7c0c2cefa commit 3ca3e6e16df706867db79f92e81246c7c0c2cefa Author: Shen-En Shih <petershih@google.com> Date: Thu Jul 27 13:00:46 2017
,
Jul 28 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/factory/+/162424670930cfe03b5c3348c878b130647ab382 commit 162424670930cfe03b5c3348c878b130647ab382 Author: Shen-En Shih <petershih@chromium.org> Date: Fri Jul 28 10:26:42 2017 pytests: Remove deprecated pytests. Remove following pytests: * ec_lightbar * ectool_i2c_dev_id * hwmon_probe * hwmon_write * rf_radiated * sensor_movement * wifi_rf * wifi_throughput_host BUG= chromium:748540 TEST=make test Change-Id: I439d364623fda81d8b33710bcfdbf19b9c02e690 Reviewed-on: https://chromium-review.googlesource.com/588872 Commit-Ready: Shen-En Shih <petershih@chromium.org> Tested-by: Shen-En Shih <petershih@chromium.org> Reviewed-by: Chih-Yu Huang <akahuang@chromium.org> Reviewed-by: Ting Shen <phoenixshen@chromium.org> [delete] https://crrev.com/ffad996c87ce61a95e05cedc8c266506707dc1b8/py/test/pytests/ec_lightbar.py [delete] https://crrev.com/ffad996c87ce61a95e05cedc8c266506707dc1b8/py/test/pytests/ectool_i2c_dev_id.py [modify] https://crrev.com/162424670930cfe03b5c3348c878b130647ab382/py/test/test_lists/generic_smt.py [delete] https://crrev.com/ffad996c87ce61a95e05cedc8c266506707dc1b8/py/test/pytests/wifi_rf.py [modify] https://crrev.com/162424670930cfe03b5c3348c878b130647ab382/po/zh-CN.po [delete] https://crrev.com/ffad996c87ce61a95e05cedc8c266506707dc1b8/py/test/pytests/rf_radiated/factory_common.py [delete] https://crrev.com/ffad996c87ce61a95e05cedc8c266506707dc1b8/py/test/pytests/rf_radiated/wifi_radiated_config.sample.yaml [delete] https://crrev.com/ffad996c87ce61a95e05cedc8c266506707dc1b8/py/test/pytests/sensor_movement.py [delete] https://crrev.com/ffad996c87ce61a95e05cedc8c266506707dc1b8/py/test/pytests/rf_radiated/rf_radiated.py [delete] https://crrev.com/ffad996c87ce61a95e05cedc8c266506707dc1b8/py/test/pytests/hwmon_write.py [delete] https://crrev.com/ffad996c87ce61a95e05cedc8c266506707dc1b8/py/test/pytests/hwmon_probe.py [delete] https://crrev.com/ffad996c87ce61a95e05cedc8c266506707dc1b8/py/test/pytests/wifi_throughput_host.py [delete] https://crrev.com/ffad996c87ce61a95e05cedc8c266506707dc1b8/py/test/pytests/rf_radiated/__init__.py [delete] https://crrev.com/ffad996c87ce61a95e05cedc8c266506707dc1b8/py/test/pytests/rf_radiated/lte_radiated_config.sample.yaml
,
Aug 1 2017
The rest part for this issue is: Following pytests might need to be renamed: 1. [#17] als_fixture 2. [#17] light_sensor 3. [#18] *_battery to battery_* I'd like to know is there any rule of thumb about the naming of the pytests? And, any idea/decision on how to rename these pytests?
,
Aug 1 2017
> any rule of thumb about the naming of the pytests? No, but I think it would be easier to read when converted to label on test list. > And, any idea/decision on how to rename these pytests? My two cents: 1. It's easier to find "related test" if they have same prefix, i.e., battery_* are better than *_battery. 2. ALS (Ambient Light Sensor) is one kind of light sensor, but currently on laptops we usually only have ALS. So "light sensor" or "ambient light sensor" both sound ok to me, since our test code is actually more generic for any kinds of light sensors. So maybe light_sensor and light_sensor_calibration (or light_sensor_fixture). I personally feel light_sensor_calibration better since that tells us what it does instead of how (fixture) it does.
,
Aug 7 2017
The following revision refers to this bug: https://chrome-internal.googlesource.com/chromeos/overlays/overlay-samus-private/+/bf4f9ec243b9ebe093d9419a50a87d9b4101e2c4 commit bf4f9ec243b9ebe093d9419a50a87d9b4101e2c4 Author: Shen-En Shih <petershih@google.com> Date: Mon Aug 07 12:51:15 2017
,
Aug 7 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/factory/+/265677522acefac24bc7d10d655e9eabfecf3e0e commit 265677522acefac24bc7d10d655e9eabfecf3e0e Author: Shen-En Shih <petershih@chromium.org> Date: Mon Aug 07 18:45:04 2017 pytests: Rename 'sysfs_battery' to 'battery_sysfs' Rename the pytest from 'sysfs_battery' to 'battery_sysfs'. This groups the battery-related test together when sorting alphabetically. BUG= chromium:748540 TEST=make test Change-Id: I76678f3bb237b1d188574e4981ab62753d6c4029 Reviewed-on: https://chromium-review.googlesource.com/602106 Commit-Ready: Shen-En Shih <petershih@chromium.org> Tested-by: Shen-En Shih <petershih@chromium.org> Reviewed-by: Shen-En Shih <petershih@chromium.org> [modify] https://crrev.com/265677522acefac24bc7d10d655e9eabfecf3e0e/po/zh-CN.po [modify] https://crrev.com/265677522acefac24bc7d10d655e9eabfecf3e0e/py/test/test_lists/generic_smt.py [rename] https://crrev.com/265677522acefac24bc7d10d655e9eabfecf3e0e/py/test/pytests/battery_sysfs.py
,
Aug 9 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/factory/+/4908034c20e2178a9dcd3159fb411b496cd9bd01 commit 4908034c20e2178a9dcd3159fb411b496cd9bd01 Author: Shen-En Shih <petershih@chromium.org> Date: Wed Aug 09 09:23:17 2017 pytests: Rename 'simple_battery' to 'battery_basic' Rename the pytest from 'simple_battery' to 'battery_basic'. This will group the battery-related tests together, when sorting alphabetically. BUG= chromium:748540 TEST=make test Change-Id: Idbe5b601be3999eb1e6c62ca660cecd2c5861794 Reviewed-on: https://chromium-review.googlesource.com/603036 Commit-Ready: Shen-En Shih <petershih@chromium.org> Tested-by: Shen-En Shih <petershih@chromium.org> Reviewed-by: Hung-Te Lin <hungte@chromium.org> [modify] https://crrev.com/4908034c20e2178a9dcd3159fb411b496cd9bd01/po/zh-CN.po [rename] https://crrev.com/4908034c20e2178a9dcd3159fb411b496cd9bd01/py/test/pytests/battery_basic.py
,
Aug 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/factory/+/f9ddb06fbca6707883626b4824ebb772c73d690d commit f9ddb06fbca6707883626b4824ebb772c73d690d Author: Peter Shih <pihsun@chromium.org> Date: Mon Aug 14 12:54:36 2017 pytests: Remove mlb_version. The board version doesn't starts with build phase anymore in newer platform, and the expected_version check can be replaced using probe pytest. So we can safely remove this test. BUG= chromium:748540 TEST=make doc TEST=make test Change-Id: I100175564e4c61523a1c699ece1c2d4370675008 Reviewed-on: https://chromium-review.googlesource.com/612925 Commit-Ready: Pi-Hsun Shih <pihsun@chromium.org> Tested-by: Pi-Hsun Shih <pihsun@chromium.org> Reviewed-by: Hung-Te Lin <hungte@chromium.org> [modify] https://crrev.com/f9ddb06fbca6707883626b4824ebb772c73d690d/po/zh-CN.po [delete] https://crrev.com/7113bc0f71cb85fd9103da1b29a1dadcbe845ced/py/test/pytests/mlb_version.py [delete] https://crrev.com/7113bc0f71cb85fd9103da1b29a1dadcbe845ced/py/test/pytests/mlb_version_unittest.py
,
Aug 22 2017
add one more - verify_touch_device_fw.py should be replaced by probe as well, I think.
,
Sep 26 2017
Do we have anything left? If not, Shen-en, feel free to close the issue.
,
Sep 27 2017
#31, verify_touch_device_fw.py should be replaced by the new probe framework as well. This issue can be tracked in this CL: https://bugs.chromium.org/p/chromium/issues/detail?id=677867#c11 #32, Yes, all mentioned cleanups are done now.
,
Oct 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/factory/+/370e7ac4f0d1bb7bee8b4412c6ce15bc2e1fcf1f commit 370e7ac4f0d1bb7bee8b4412c6ce15bc2e1fcf1f Author: Peter Shih <pihsun@chromium.org> Date: Mon Oct 16 04:55:55 2017 pytests: Merge display_idle with display. The display_idle pytest is not used by anyone, and the code are most duplicated from display pytest. To avoid double effort to maintain these two tests, we can delete the display_idle, and add an argument for display pytest to just show an image for given duration and pass. BUG= chromium:748540 TEST=make test Change-Id: Ie9ca60068affaace5c91f1c034dc894f4527288e Reviewed-on: https://chromium-review.googlesource.com/717997 Commit-Ready: Pi-Hsun Shih <pihsun@chromium.org> Tested-by: Pi-Hsun Shih <pihsun@chromium.org> Reviewed-by: Hung-Te Lin <hungte@chromium.org> [modify] https://crrev.com/370e7ac4f0d1bb7bee8b4412c6ce15bc2e1fcf1f/po/zh-CN.po [delete] https://crrev.com/32d3316afb52b751db1733ee5857da406d01da9d/py/test/pytests/display_idle_static/display_idle.js [modify] https://crrev.com/370e7ac4f0d1bb7bee8b4412c6ce15bc2e1fcf1f/py/test/pytests/display.py [delete] https://crrev.com/32d3316afb52b751db1733ee5857da406d01da9d/py/test/pytests/display_idle_static/display_idle.css [delete] https://crrev.com/32d3316afb52b751db1733ee5857da406d01da9d/py/test/pytests/display_idle.py [modify] https://crrev.com/370e7ac4f0d1bb7bee8b4412c6ce15bc2e1fcf1f/py/test/pytests/display_static/display.js
,
Jan 22 2018
,
Jan 23 2018
|
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by hungte@chromium.org
, Jul 26 2017Status: Assigned (was: Untriaged)