Merge lp:~sylvain-pineau/checkbox/fix-1561821 into lp:checkbox

Proposed by Sylvain Pineau
Status: Merged
Approved by: Sylvain Pineau
Approved revision: 4290
Merged at revision: 4290
Proposed branch: lp:~sylvain-pineau/checkbox/fix-1561821
Merge into: lp:checkbox
Diff against target: 68 lines (+11/-4)
3 files modified
plainbox/plainbox/impl/unit/template.py (+11/-0)
providers/plainbox-provider-checkbox/jobs/input.txt.in (+0/-2)
providers/plainbox-provider-checkbox/jobs/suspend.txt.in (+0/-2)
To merge this branch: bzr merge lp:~sylvain-pineau/checkbox/fix-1561821
Reviewer Review Type Date Requested Status
Pierre Equoy Approve
Sylvain Pineau (community) Needs Resubmitting
Review via email: mp+290320@code.launchpad.net

Description of the change

Fixes the linked bug, but without modifying the udev parser to add an extra input-id field.

To post a comment you must log in.
Revision history for this message
Pierre Equoy (pieq) wrote :

It's very smart!

When I was testing it and checking how it worked, I realized there are other jobs where we need to remove the {path} parameter in the description or else it will fail the same way:

  - suspend/pointing-after-suspend_{product_slug}_{category}_{__index__}
  - suspend/clicking-after-suspend_{product_slug}_{category}_{__index__}

Other than that, it looks great. Thanks!

review: Needs Fixing
4290. By Sylvain Pineau

providers:checkbox: Remove {path} requirement in input jobs

As those may change if the same device is unplugged/re-plugged
(even on the same port).

Revision history for this message
Sylvain Pineau (sylvain-pineau) wrote :

I've fixed the suspend jobs

review: Needs Resubmitting
Revision history for this message
Pierre Equoy (pieq) wrote :

+1

Thanks!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'plainbox/plainbox/impl/unit/template.py'
--- plainbox/plainbox/impl/unit/template.py 2016-03-03 16:04:58 +0000
+++ plainbox/plainbox/impl/unit/template.py 2016-03-30 08:11:28 +0000
@@ -32,6 +32,7 @@
32from plainbox.impl.secure.origin import Origin32from plainbox.impl.secure.origin import Origin
33from plainbox.impl.symbol import SymbolDef33from plainbox.impl.symbol import SymbolDef
34from plainbox.impl.unit import all_units34from plainbox.impl.unit import all_units
35from plainbox.impl.unit import get_accessed_parameters
35from plainbox.impl.unit._legacy import TemplateUnitLegacyAPI36from plainbox.impl.unit._legacy import TemplateUnitLegacyAPI
36from plainbox.impl.unit._legacy import TemplateUnitValidatorLegacyAPI37from plainbox.impl.unit._legacy import TemplateUnitValidatorLegacyAPI
37from plainbox.impl.unit.unit import Unit38from plainbox.impl.unit.unit import Unit
@@ -366,6 +367,16 @@
366 # XXX: extract raw dictionary from the resource object, there is no367 # XXX: extract raw dictionary from the resource object, there is no
367 # normal API for that due to the way resource objects work.368 # normal API for that due to the way resource objects work.
368 parameters = dict(object.__getattribute__(resource, '_data'))369 parameters = dict(object.__getattribute__(resource, '_data'))
370 accessed_parameters = set(itertools.chain(*{
371 get_accessed_parameters(value) for value in data.values()}))
372 # Recreate the parameters with only the subset that will actually be
373 # used by the template. Doing this filter can prevent exceptions like
374 # DependencyDuplicateError where an unused resource property can differ
375 # when resuming and bootstrapping sessions, causing job checksums
376 # mismatches.
377 # See https://bugs.launchpad.net/bugs/1561821
378 parameters = {
379 k: v for k, v in parameters.items() if k in accessed_parameters}
369 # Add the special __index__ to the resource namespace variables380 # Add the special __index__ to the resource namespace variables
370 parameters['__index__'] = index381 parameters['__index__'] = index
371 # Instantiate the class using the instantiation API382 # Instantiate the class using the instantiation API
372383
=== modified file 'providers/plainbox-provider-checkbox/jobs/input.txt.in'
--- providers/plainbox-provider-checkbox/jobs/input.txt.in 2016-03-02 14:01:49 +0000
+++ providers/plainbox-provider-checkbox/jobs/input.txt.in 2016-03-30 08:11:28 +0000
@@ -5,7 +5,6 @@
5category_id: 2013.com.canonical.plainbox::input5category_id: 2013.com.canonical.plainbox::input
6id: input/pointing_{product_slug}_{category}_{__index__}6id: input/pointing_{product_slug}_{category}_{__index__}
7estimated_duration: 30.07estimated_duration: 30.0
8requires: device.path == '{path}'
9_summary: Check pointing functionality for {product}8_summary: Check pointing functionality for {product}
10_purpose:9_purpose:
11 This will test your {product} device10 This will test your {product} device
@@ -78,7 +77,6 @@
78category_id: 2013.com.canonical.plainbox::input77category_id: 2013.com.canonical.plainbox::input
79id: input/clicking_{product_slug}_{category}_{__index__}78id: input/clicking_{product_slug}_{category}_{__index__}
80estimated_duration: 30.079estimated_duration: 30.0
81requires: device.path == '{path}'
82_summary: Check button functionality for {product}80_summary: Check button functionality for {product}
83_purpose:81_purpose:
84 This will test the buttons of your {product} device82 This will test the buttons of your {product} device
8583
=== modified file 'providers/plainbox-provider-checkbox/jobs/suspend.txt.in'
--- providers/plainbox-provider-checkbox/jobs/suspend.txt.in 2016-03-11 14:36:34 +0000
+++ providers/plainbox-provider-checkbox/jobs/suspend.txt.in 2016-03-30 08:11:28 +0000
@@ -2673,7 +2673,6 @@
2673category_id: 2013.com.canonical.plainbox::suspend2673category_id: 2013.com.canonical.plainbox::suspend
2674id: suspend/pointing-after-suspend_{product_slug}_{category}_{__index__}2674id: suspend/pointing-after-suspend_{product_slug}_{category}_{__index__}
2675depends: suspend/suspend_advanced2675depends: suspend/suspend_advanced
2676requires: device.path == "{path}"
2677_description:2676_description:
2678 PURPOSE:2677 PURPOSE:
2679 This will test your {product} device after suspend.2678 This will test your {product} device after suspend.
@@ -2690,7 +2689,6 @@
2690estimated_duration: 30.02689estimated_duration: 30.0
2691id: suspend/clicking-after-suspend_{product_slug}_{category}_{__index__}2690id: suspend/clicking-after-suspend_{product_slug}_{category}_{__index__}
2692depends: suspend/suspend_advanced2691depends: suspend/suspend_advanced
2693requires: device.path == "{path}"
2694_summary: Check post suspend button functionality for {product}2692_summary: Check post suspend button functionality for {product}
2695_description:2693_description:
2696 PURPOSE:2694 PURPOSE:

Subscribers

People subscribed via source and target branches