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

Issue 695784 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Last visit > 30 days ago
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Feature



Sign in to add a comment

factory: merge "touchscreen" and "touchscreen_wrap" tests

Project Member Reported by youcheng@chromium.org, Feb 24 2017

Issue description

TODO:
1. make "touchscreen_wrap" support evdev touchscreen events
2. make "touchscreen_wrap" support evdev stylus events (touching and hovering)
3. add time limit to "touchscreen_wrap"
4. make touchscreen_wrap support arbitrary order (in addition to spiral order),
   and deprecate "touchscreen" test
 
Current "touchscreen_wrap" test is an end-to-end test, which reads touchscreen events via HTML5 touch API.
There is another test named "touchscreen", that reads events from evdev.
Moreover, operators need to touch the panel in spiral order for "touchscreen_wrap" test,
while "touchscreen" test doesn't require any particular pattern.

We'd like to merge these two tests into one single test.
Cc: hungte@chromium.org josephsih@chromium.org
Hi Joseph,

To merge these two tests, I wrote a new module "touch_state" in http://crosreview.com/424276

This module can be used to fetch states and monitor events of touch devices.
Furthermore, we can simplify touchpad test and stylus test with this module like this:
http://crosreview.com/448436

Module "touch_utils", originally written by you, is doing similar things but is more like static analysing evtest log files,
so it is hard for me to integrate touch_state into touch_utils.
And after CL:448436, no one will use touch_utils anymore.
What do you think about touch_utils?
Should we keep it in factory source tree?
Project Member

Comment 3 by bugdroid1@chromium.org, May 18 2017

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

commit 9f0012eac8c95f778117efd6f8caffe0ff70b577
Author: You-Cheng Syu <youcheng@chromium.org>
Date: Thu May 18 09:26:11 2017

touchscreen_wrap: support evdev touchscreen and stylus

To merge "touchscreen" and "touchscreen_wrap" tests, firstly we need to
make "touchscreen_wrap" support reading events from evdev as
"touchscreen" test.

BUG= chromium:695784 
TEST=manually on kevin

Change-Id: Ied8c4df9edaa033bcdab8a556ad2bad9bc04e879
Reviewed-on: https://chromium-review.googlesource.com/424276
Commit-Ready: Youcheng Syu <youcheng@chromium.org>
Tested-by: Youcheng Syu <youcheng@chromium.org>
Reviewed-by: Shuo-Peng Liao <deanliao@chromium.org>

[modify] https://crrev.com/9f0012eac8c95f778117efd6f8caffe0ff70b577/py/test/pytests/touchscreen_wrap.py
[modify] https://crrev.com/9f0012eac8c95f778117efd6f8caffe0ff70b577/py/test/pytests/touchscreen_wrap_static/touchscreen_wrap.js
[modify] https://crrev.com/9f0012eac8c95f778117efd6f8caffe0ff70b577/py/test/utils/evdev_utils.py
[add] https://crrev.com/9f0012eac8c95f778117efd6f8caffe0ff70b577/py/test/utils/touch_monitor.py

Comment 4 by hungte@chromium.org, Jul 26 2017

Cc: -hungte@chromium.org chromeos-factory-eng@google.com
Hey youcheng, will we still merge the tests , or you have new thoughts?
Yes, I'm going to merge them in the following days.
I've uploaded CL:585992 to make touchscreen_wrap test support arbitrary order.
Now the only thing that touchscreen test supports but touchscreen_wrap doesn't is the time limit.
However, I'm wondering if we really needs time limit.
If not, then we can remove touchscreen test after the CL merged.

Comment 7 by pihsun@chromium.org, Jul 26 2017

The timeout for touchscreen is because that on detachable device, if there's no timeout, and the touchscreen is really broken, there would be no way to fail the test by operator.
Ok. Sounds like we must provide a timeout mechanism.

Comment 9 by hungte@chromium.org, Jul 26 2017

can we keep the primary implementation as "touchscreen" and remove (or make it a symlink) touchscreen_wrap?

I think it's better to use simple words for test name.
Project Member

Comment 10 by bugdroid1@chromium.org, Jul 27 2017

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

commit a053eaf1e94cf8ae4b7bd092aec3e98317d9ff1b
Author: You-Cheng Syu <youcheng@chromium.org>
Date: Thu Jul 27 11:22:40 2017

pytests: Make touchscreen_wrap support arbitrary order mode.

To merge touchscreen_wrap and touchscreen tests, we want to make
touchscreen_wrap test support most features that touchscreen test
provide.

Previously operators must draw block in spiral order to pass
touchscreen_wrap test, this change add an argument 'spiral_mode' to
support arbitrary order mode as touchscreen test.

Also improve the document.

BUG= chromium:695784 
TEST=manually

Change-Id: Id7ded432d927c4959913565d35635cfbdb3a6e7a
Reviewed-on: https://chromium-review.googlesource.com/585992
Commit-Ready: Youcheng Syu <youcheng@chromium.org>
Tested-by: Youcheng Syu <youcheng@chromium.org>
Reviewed-by: Pi-Hsun Shih <pihsun@chromium.org>

[modify] https://crrev.com/a053eaf1e94cf8ae4b7bd092aec3e98317d9ff1b/po/zh-CN.po
[modify] https://crrev.com/a053eaf1e94cf8ae4b7bd092aec3e98317d9ff1b/py/test/pytests/touchscreen_wrap.py
[modify] https://crrev.com/a053eaf1e94cf8ae4b7bd092aec3e98317d9ff1b/py/test/pytests/touchscreen_wrap_static/touchscreen_wrap.js

Project Member

Comment 11 by bugdroid1@chromium.org, Jul 27 2017

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

commit f9a51689dfc5b07dd56597f99db42720f2d9ee90
Author: You-Cheng Syu <youcheng@chromium.org>
Date: Thu Jul 27 11:22:40 2017

pytests: Make touchscreen_wrap support timeout mechanism.

Add an optional countdown timer that can forcely fail the test if it's
not passed in a given time limit.

BUG= chromium:695784 
TEST=manually

Change-Id: Icebb7c8612c2eb8770fbdd80d175ccfa3ec8b483
Reviewed-on: https://chromium-review.googlesource.com/586209
Commit-Ready: Youcheng Syu <youcheng@chromium.org>
Tested-by: Youcheng Syu <youcheng@chromium.org>
Reviewed-by: Pi-Hsun Shih <pihsun@chromium.org>

[modify] https://crrev.com/f9a51689dfc5b07dd56597f99db42720f2d9ee90/py/test/pytests/touchscreen_wrap.py
[modify] https://crrev.com/f9a51689dfc5b07dd56597f99db42720f2d9ee90/py/test/pytests/touchscreen_wrap_static/touchscreen_wrap.js
[modify] https://crrev.com/f9a51689dfc5b07dd56597f99db42720f2d9ee90/py/test/pytests/touchscreen_wrap_static/touchscreen_wrap.css

Project Member

Comment 12 by bugdroid1@chromium.org, Jul 28 2017

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

commit 2522db0c967a5bc000bf3f0f8f7b732637217a98
Author: You-Cheng Syu <youcheng@chromium.org>
Date: Fri Jul 28 12:45:00 2017

pytests: Replace touchscreen test with touchscreen_wrap test.

touchscreen_wrap test now provides more features than touchscreen test,
so there is no reason to keep touchscreen test now.

This change removes old touchscreen test, renames touchscreen_wrap to
touchscreen, and set default value to spiral_mode=True and
timeout_secs=20.

BUG= chromium:695784 
TEST=manually

Change-Id: Ic68542cbc3e558923cdb71daa65966b424e6cbe4
Reviewed-on: https://chromium-review.googlesource.com/586350
Commit-Ready: Youcheng Syu <youcheng@chromium.org>
Tested-by: Youcheng Syu <youcheng@chromium.org>
Reviewed-by: Pi-Hsun Shih <pihsun@chromium.org>

[delete] https://crrev.com/0b105595565c9e508a91ce57be13d0e063660330/py/test/pytests/touchscreen_wrap_static/touchscreen_wrap.js
[modify] https://crrev.com/2522db0c967a5bc000bf3f0f8f7b732637217a98/py/test/pytests/touchscreen_static/touchscreen.css
[modify] https://crrev.com/2522db0c967a5bc000bf3f0f8f7b732637217a98/po/zh-CN.po
[delete] https://crrev.com/0b105595565c9e508a91ce57be13d0e063660330/py/test/pytests/touchscreen_wrap_static/touchscreen_wrap.css
[modify] https://crrev.com/2522db0c967a5bc000bf3f0f8f7b732637217a98/py/test/pytests/touchscreen_static/touchscreen.js
[modify] https://crrev.com/2522db0c967a5bc000bf3f0f8f7b732637217a98/py/test/pytests/touchscreen.py
[delete] https://crrev.com/0b105595565c9e508a91ce57be13d0e063660330/py/test/pytests/touchscreen_wrap.py

Status: Fixed (was: Started)

Comment 14 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Sign in to add a comment