Merge lp:~cgregan/checkbox/hybrid-graphics-screenshot into lp:checkbox

[Chris Gregan]

Creation of 2 new jobs in Graphics:
graphics/generator_screen-capture-internal
graphics/generator_screen-capture-internal.png

[Zygmunt Krynicki (zyga)]

Please don't change permissions of existing files.

review: Needs Fixing

[Zygmunt Krynicki (zyga)]

I wonder if we could use templates for that job as well.

[Zygmunt Krynicki (zyga)]

Looking at the output of that script, I think we can use templates for this job.

PATH=./providers/plainbox-provider-resource-generic/bin:$PATH ./providers/plainbox-provider-resource-generic/bin/graphics_card_resource
bus: pci
category: VIDEO
driver: i915
index: 1
path: /devices/pci0000:00/0000:00:02.0
product: 3rd Gen Core processor Graphics Controller
product_id: 358
subproduct_id: 14711
subvendor_id: 6058
vendor: Intel Corporation
vendor_id: 32902

[Zygmunt Krynicki (zyga)]

I see three units: the resource job that runs graphics_card_resource, the template that does the screenshot and the template that attaches it. (untested). This also fixes a few issues present in the original jobs (bad references between jobs, duplicate keys, etc.

unit: job
id: graphics_card_resource
plugin: resource
command: graphics_card_resource

unit: template
template-unit: job
template-resource: graphics_card_resource
plugin: shell
id: graphics/{index}_screen-capture-internal_{product}
_summary: Obtains a simple screen capture of {product}
estimated_duration: 1.0
requires: package.name == 'gnome-screenshot'
command: gnome-screenshot --file ${PLAINBOX_SESSION_SHARE}/screen-capture-{index}.png
_description: Obtains a simple screen capture of {product} using gnome-screenshot if present

unit: template
template-unit: job
template-resource: graphics_card_resource
plugin: attachment
id: graphics/${index}_screen-capture-internal_{product}
depends: graphics/${index}_screen-capture-internal_{product}
_summary: Attaches a simple screen capture of {product}
estimated_duration: 1.0
command: [ -f ${PLAINBOX_SESSION_SHARE}/screen-capture-{index}.png ] && base64 ${PLAINBOX_SESSION_SHARE}/screen-capture-{index}.png
_description: Attaches the simple screen capture file of {product}

[Daniel Manrique (roadmr)]

woot, are template units now ready to be used? yay

[lp:~cgregan/checkbox/hybrid-graphics-screenshot]

3591. By Chris Gregan on 2015-02-25

Updated the new hybrid graphics jobs to use templates

[Daniel Manrique (roadmr)]

This looks OK, however it fails:

$ plainbox run -i 2013.com.canonical.certification::graphics_card_resource -i 2013.com.canonical.certification::graphics/1_screen.*
ERROR plainbox.ctrl: Ignoring invalid instantiated unit Obtains a simple screen capture of Haswell-ULT Integrated Graphics Controller: Problem with field command: wrong
ERROR plainbox.ctrl: Ignoring invalid instantiated unit Attaches a simple screen capture of Haswell-ULT Integrated Graphics Controller: Problem with field command: wrong
Outcome: job passed
==================================[ Results ]===================================
 ☑ : graphics_card_resource

The reason is that the command mentions ${PLAINBOX_SESSION_SHARE}, and I suspect plainbox is erroneously trying to interpolate {PLAINBOX_SESSION_SHARE} as if it came from the resource; since the resource doesn't provide this, substitution obviously fails.

I tested the theory by replacing the variable syntax to just $PLAINBOX_SESSION_SHARE and it worked.

I filed https://bugs.launchpad.net/plainbox/+bug/1425707, but in the meanwhile the suggested workaround is to fix $PLAINBOX_SESSION_SHARE as mentioned above. I'll set to Needs Fixing since, as is, it doesn't work (though it's because of the plainbox bug).

review: Needs Fixing

[Zygmunt Krynicki (zyga)]

Yes, that is correct. In templates to type { use {{ and ditto for }.

Thanks for catching this!

On Wed, Feb 25, 2015 at 10:38 PM, Daniel Manrique
<email address hidden> wrote:
> Review: Needs Fixing
>
> This looks OK, however it fails:
>
> $ plainbox run -i 2013.com.canonical.certification::graphics_card_resource -i 2013.com.canonical.certification::graphics/1_screen.*
> ERROR plainbox.ctrl: Ignoring invalid instantiated unit Obtains a simple screen capture of Haswell-ULT Integrated Graphics Controller: Problem with field command: wrong
> ERROR plainbox.ctrl: Ignoring invalid instantiated unit Attaches a simple screen capture of Haswell-ULT Integrated Graphics Controller: Problem with field command: wrong
> Outcome: job passed
> ==================================[ Results ]===================================
> ☑ : graphics_card_resource
>
>
> The reason is that the command mentions ${PLAINBOX_SESSION_SHARE}, and I suspect plainbox is erroneously trying to interpolate {PLAINBOX_SESSION_SHARE} as if it came from the resource; since the resource doesn't provide this, substitution obviously fails.
>
> I tested the theory by replacing the variable syntax to just $PLAINBOX_SESSION_SHARE and it worked.
>
> I filed https://bugs.launchpad.net/plainbox/+bug/1425707, but in the meanwhile the suggested workaround is to fix $PLAINBOX_SESSION_SHARE as mentioned above. I'll set to Needs Fixing since, as is, it doesn't work (though it's because of the plainbox bug).
>
> --
> https://code.launchpad.net/~cgregan/checkbox/hybrid-graphics-screenshot/+merge/250945
> You are reviewing the proposed merge of lp:~cgregan/checkbox/hybrid-graphics-screenshot into lp:checkbox.

[lp:~cgregan/checkbox/hybrid-graphics-screenshot]

3592. By Chris Gregan on 2015-02-25

updated $PLAINBOX_SESSION_SHARE

3593. By Chris Gregan on 2015-02-25

Removing eroneous $ from depends and id lines

[Daniel Manrique (roadmr)]

OK, two more things.

The id for the attachment job is still weird, as it's identical to the screenshot job. I suspect the screenshot needs a .png at the end of the id.

Also, this won't work yet due to bug 1425722 which I discovered while testing this. A fix for that is proposed:

https://code.launchpad.net/~roadmr/checkbox/1425722-graphics-card-resource/+merge/251016

I'll mark this merge as depending on that one, but don't forget to commit the id fix :)

review: Needs Fixing

[Daniel Manrique (roadmr)]

oops, actually I can't add a prerequisite branch :( so let's just hold this until the bug fix is merged.

[Daniel Manrique (roadmr)]

Hi Chris,

Zygmunt ended up doing a different fix, so the only change needed here is to quote the job name in the dependency, because {product} can contain spaces. See inline comment in the affected line.

Once that's done, we should be OK to merge.

review: Needs Fixing

[lp:~cgregan/checkbox/hybrid-graphics-screenshot]

3594. By Chris Gregan on 2015-02-26

updated the depends call

[Chris Gregan (cgregan)]

Done.

[Daniel Manrique (roadmr)]

Oh,sorry! one *final* change needed (I tested it locally and it's the only missing bit). see below...

review: Needs Fixing

[Daniel Manrique (roadmr)]

Sorry, here's the required change.

[Chris Gregan (cgregan)]

Ah..I wondered when I committed, but then skipped because I was not sure if
it applied to id and depends or just depends

On Thu, Feb 26, 2015 at 3:06 PM, Daniel Manrique <
<email address hidden>> wrote:

> Sorry, here's the required change.
>
> Diff comments:
>
> > === modified file 'providers/plainbox-provider-checkbox/jobs/
> graphics.txt.in'
> > --- providers/plainbox-provider-checkbox/jobs/graphics.txt.in
> 2015-02-03 11:05:57 +0000
> > +++ providers/plainbox-provider-checkbox/jobs/graphics.txt.in
> 2015-02-26 19:08:18 +0000
> > @@ -550,3 +550,30 @@
> > _description: Attaches the simple screen capture file
> >
> >
> > +unit: job
> > +id: graphics_card_resource
> > +plugin: resource
> > +command: graphics_card_resource
> > +
> > +unit: template
> > +template-unit: job
> > +template-resource: graphics_card_resource
> > +plugin: shell
> > +id: graphics/{index}_screen-capture-internal_{product}
> > +_summary: Obtains a simple screen capture of {product}
> > +estimated_duration: 1.0
> > +requires: package.name == 'gnome-screenshot'
> > +command: gnome-screenshot --file
> $PLAINBOX_SESSION_SHARE/screen-capture-{index}.png
> > +_description: Obtains a simple screen capture of {product} using
> gnome-screenshot if present
> > +
> > +unit: template
> > +template-unit: job
> > +template-resource: graphics_card_resource
> > +plugin: attachment
> > +id: graphics/{index}_screen-capture-internal_{product}
>
> The ID is the same as the above job. CHange to:
>
> id: graphics/{index}_screen-capture-internal_{product}.png
>
> > +depends: "graphics/{index}_screen-capture-internal_{product}"
> > +_summary: Attaches a simple screen capture of {product}
> > +estimated_duration: 1.0
> > +command: [ -f $PLAINBOX_SESSION_SHARE/screen-capture-{index}.png ] &&
> base64 $PLAINBOX_SESSION_SHARE/screen-capture-{index}.png
> > +_description: Attaches the simple screen capture file of {product}
> > +
> >
>
>
> --
>
> https://code.launchpad.net/~cgregan/checkbox/hybrid-graphics-screenshot/+merge/250945
> You are the owner of lp:~cgregan/checkbox/hybrid-graphics-screenshot.
>

--
Chris Gregan
Project Manager
Professional and Engineering Services
Canonical USA Inc.
<email address hidden>
cgregan[irc.freenode.net]
W-781-761-9448

----
1024/8806032D
E70F 7391 6C78 9B9E 6461 1CC7 B168 E1E7 8806 032D

[lp:~cgregan/checkbox/hybrid-graphics-screenshot]

3595. By Chris Gregan on 2015-02-26

typo for id of attachment

[Daniel Manrique (roadmr)]

Thanks, let's merge this!

review: Approve

=== modified file 'providers/plainbox-provider-checkbox/jobs/graphics.txt.in'
--- providers/plainbox-provider-checkbox/jobs/graphics.txt.in	2015-02-03 11:05:57 +0000
+++ providers/plainbox-provider-checkbox/jobs/graphics.txt.in	2015-02-26 20:21:18 +0000
@@ -550,3 +550,30 @@
 _description: Attaches the simple screen capture file
 
 
+unit: job
+id: graphics_card_resource
+plugin: resource
+command: graphics_card_resource
+
+unit: template
+template-unit: job
+template-resource: graphics_card_resource
+plugin: shell
+id: "graphics/{index}_screen-capture-internal_{product}"
+_summary: Obtains a simple screen capture of {product}
+estimated_duration: 1.0
+requires: package.name == 'gnome-screenshot'
+command: gnome-screenshot --file $PLAINBOX_SESSION_SHARE/screen-capture-{index}.png
+_description: Obtains a simple screen capture of {product} using gnome-screenshot if present
+
+unit: template
+template-unit: job
+template-resource: graphics_card_resource
+plugin: attachment
+id: graphics/{index}_screen-capture-internal_{product}.png
+depends: "graphics/{index}_screen-capture-internal_{product}"
+_summary: Attaches a simple screen capture of {product}
+estimated_duration: 1.0
+command: [ -f $PLAINBOX_SESSION_SHARE/screen-capture-{index}.png ] && base64 $PLAINBOX_SESSION_SHARE/screen-capture-{index}.png
+_description: Attaches the simple screen capture file of {product}
+