Checkbox

Merge lp://staging/~cr3/checkbox/touchpad_scroll_resource_deprecate into lp://staging/checkbox

touchpad_scroll_resource_deprecate
Merge into trunk

Proposed by Marc Tardif on 2012-10-19

Status:	Merged
Merged at revision:	1791
Proposed branch:	lp://staging/~cr3/checkbox/touchpad_scroll_resource_deprecate
Merge into:	lp://staging/checkbox
Diff against target:	252 lines (+32/-86) 8 files modified checkbox/parsers/tests/test_xinput.py (+2/-2) checkbox/parsers/xinput.py (+2/-2) data/whitelists/default.whitelist (+1/-1) debian/changelog (+2/-0) jobs/resource.txt.in (+4/-4) jobs/touchpad.txt.in (+15/-4) jobs/touchscreen.txt.in (+6/-2) scripts/touchpad_scroll_resource (+0/-71)
To merge this branch:	bzr merge lp://staging/~cr3/checkbox/touchpad_scroll_resource_deprecate
Related bugs:	Link a bug report

Reviewer	Date Requested	Status
Jeff Marcom (community)	2012-10-19	Approve on 2012-10-22
Marc Tardif (community)		Needs Resubmitting on 2012-10-22
Review via email: mp+130607@code.staging.launchpad.net

Commit message

Merged fix to touchpad scroll tests and replacement of touchpad_scroll_resource script by cr3.

Description of the change

This merge request fixes a problem noticed by spideyman where the scroll tests prompt the user on an all-in-one system. While fixing this problem, I took the opportunity to replace the touchpad_scroll_resource script to reuse the more general xinput_resource script.

Revision history for this message

Jeff Marcom (jeffmarcom) wrote on 2012-10-19:

Okay, so a few problems here.

1.) This works pretty well in an all-in-one config now, however my tested this on my laptop (which is capable of vertical scrolling) and the test was skipped.

2.) Please add this requires line to the automated multitouch touchpad test: dmi.product in ['Notebook','Laptop','Portable']

review: Needs Fixing

lp://staging/~cr3/checkbox/touchpad_scroll_resource_deprecate updated on 2012-10-19

1786. By Marc Tardif on 2012-10-19: Fixed strings in touchpad/vertical requires line.
1787. By Marc Tardif on 2012-10-19: Added requires line to touchpad/multitouch-automated.

Revision history for this message

Marc Tardif (cr3) wrote on 2012-10-19:

For 1, you're absolutely right, fixed!

For 2, I left that out on purpose because it would automatically report fail if there was no touchpad. However, it would probably make more sense for that test to report unsupported. Also fixed.

review: Needs Resubmitting

Revision history for this message

Jeff Marcom (jeffmarcom) wrote on 2012-10-19:

> For 1, you're absolutely right, fixed!
>
> For 2, I left that out on purpose because it would automatically report fail
> if there was no touchpad. However, it would probably make more sense for that
> test to report unsupported. Also fixed.

Thanks, and for 2 I agree. I would rather see it say unsupported for both single and multi. It shouldn't fail if it doesn't meet the criteria for one or the other. I realize my initial request did not recover this, and I was just looking for their to be a check before initiating the test at all.

Let me know and we can have a follow up discussion on this. I'd rather have this decided before a commit.

Thanks

review: Needs Information

Revision history for this message

Jeff Marcom (jeffmarcom) wrote on 2012-10-19:

In case it wasn't clear from the above statement, it's still marking single-touch automated as "fail" on my laptop.

Revision history for this message

Marc Tardif (cr3) wrote on 2012-10-20:

What makes you say that your touchpad is not multitouch on your laptop?

lp://staging/~cr3/checkbox/touchpad_scroll_resource_deprecate updated on 2012-10-22

1788. By Marc Tardif on 2012-10-22: Renamed class to device_class in xinput parser so that it doesn't conflict with the Python name.
1789. By Marc Tardif on 2012-10-22: Improved touchpad/singletouch-automated, touchpad/multitouch-automated, touchscreen/nontouch-automated and touchscreen/multitouch-automated so that they return unsupported instead of fail when the device is not singletouch for example.

Revision history for this message

Marc Tardif (cr3) wrote on 2012-10-22:

Done, the touchpad/singletouch should now return as unsupported on your laptop. When testing again, please make sure to branch in a new directory instead of pulling the latest changes. That way, you make sure there's no cache and no store from your previous run. Thanks!

review: Needs Resubmitting

Revision history for this message

Jeff Marcom (jeffmarcom) wrote on 2012-10-22:

Awesome, thanks for your patience!

review: Approve