Merge ~bettyl/plainbox-provider-checkbox:modify-graphics-tests into plainbox-provider-checkbox:master

[Betty Lin]

Test report:
https://certification.canonical.com/hardware/201906-27116/submission/148292/

[Betty Lin (bettyl)]

@Jonathan

I think this MR can fix the two related bugs.
Please help to review, thanks!

review: Needs Resubmitting

[Jonathan Cave (jocave)]

I still think it would be good practice to split the one existing commit up in to a number of smaller commits to make it easier to follow the changes.

For instance the changing job ID suffixes from _auto to _graphics and the accompanying test plan changes could go in one commit on their are own.

Then following on from this, you could have a second commit that splits up the test plan in to smaller test plans.

One request below to not remove the job summary.

review: Needs Fixing

[Betty Lin (bettyl)]

@Jonathan

I split the commit. This one focus on suffix problem.
The others will propose MR soon.
Thanks!

review: Needs Resubmitting

[Jonathan Cave (jocave)]

Looks mostly fine. One question below..

review: Needs Information

[Betty Lin (bettyl)]

@Jonathan

Actually, I don't know why ... It was there before I modified the test plan:
https://git.launchpad.net/plainbox-provider-checkbox/tree/units/graphics/test-plan.pxu

I will move "suspend/2_resolution_before_suspend_.*_auto" to the test plan "graphics-discrete-gpu-cert-automated" in the next commit I send.

Thanks!

review: Needs Resubmitting

[Betty Lin (bettyl)]

The commit https://git.launchpad.net/~bettyl/plainbox-provider-checkbox/commit/?id=98b78890cc143c5ec39cea1319fbe2faeca28be9 is for fixing the bug: https://bugs.launchpad.net/plainbox-provider-checkbox/+bug/1832662

[Jonathan Cave (jocave)]

Ok, thanks for reworking the MR. Let's land it.

review: Approve

[Devices Certification Bot (ce-certification-qa)]

The merge was fine but running tests failed.

[xenial] [09:02:34] starting container
Device project added to xenial-testing
[xenial] [09:02:43] provisioning container
[bionic] [09:02:55] starting container
Device project added to bionic-testing
[xenial] [09:03:05] Starting tests...
[xenial] Found a test script: ./requirements/container-tests-provider-checkbox
[bionic] [09:03:07] provisioning container
[bionic] [09:03:23] Starting tests...
[bionic] Found a test script: ./requirements/container-tests-provider-checkbox
[xenial] [09:04:48] container-tests-provider-checkbox: FAIL
[xenial] output: https://paste.ubuntu.com/p/s4NsqmKjKy/
[xenial] [09:04:51] Fixing file permissions in source directory
[xenial] [09:04:51] Destroying container
[bionic] [09:05:17] container-tests-provider-checkbox: FAIL
[bionic] output: https://paste.ubuntu.com/p/CK3pxDq2Z2/
[bionic] [09:05:20] Fixing file permissions in source directory
[bionic] [09:05:20] Destroying container

review: Needs Fixing

[Betty Lin (bettyl)]

Fixed the error which reported by validation of provider

review: Needs Resubmitting

[Pierre Equoy (pieq)]

LGTM. Thanks!!!

review: Approve

[Pierre Equoy (pieq)]

OK, there was an extra space below an "estimated_duration" line, causing issues with the provider validation. Betty removed it and checked it locally, so this time it should be all good!

review: Approve

[Devices Certification Bot (ce-certification-qa)]

The merge was fine but running tests failed.

[xenial] [02:53:29] starting container
Device project added to xenial-testing
[xenial] [02:53:38] provisioning container
[xenial] [02:53:58] Starting tests...
[xenial] Found a test script: ./requirements/container-tests-provider-checkbox
[bionic] [02:54:34] starting container
Device project added to bionic-testing
[bionic] [02:54:46] provisioning container
[bionic] [02:55:02] Starting tests...
[bionic] Found a test script: ./requirements/container-tests-provider-checkbox
[bionic] [02:57:02] container-tests-provider-checkbox: FAIL
[xenial] [02:57:02] container-tests-provider-checkbox: FAIL
[bionic] output: https://paste.ubuntu.com/p/x8RJgfJTrt/
[xenial] output: https://paste.ubuntu.com/p/3PGQrB35BB/
[xenial] [02:57:05] Fixing file permissions in source directory
[bionic] [02:57:05] Fixing file permissions in source directory
[xenial] [02:57:05] Destroying container
[bionic] [02:57:05] Destroying container

review: Needs Fixing

diff --git a/units/graphics/test-plan.pxu b/units/graphics/test-plan.pxu
index 9085cb8..4537dfd 100644
--- a/units/graphics/test-plan.pxu
+++ b/units/graphics/test-plan.pxu
@@ -51,8 +51,8 @@ include:
 bootstrap_include:
  graphics_card
 nested_part:
- com.canonical.certification::graphics-discrete-gpu-cert-manual
  com.canonical.certification::graphics-discrete-gpu-cert-automated
+ com.canonical.certification::graphics-discrete-gpu-cert-manual
 
 id: graphics-discrete-gpu-cert-manual
 unit: test plan
@@ -60,7 +60,6 @@ _name: Graphics tests (discrete GPU) (Manual)
 _description:
  Graphics tests (discrete GPU) (Manual)
 include:
- graphics/2_auto_switch_card_.*             certification-status=blocker
  graphics/2_maximum_resolution_.*           certification-status=blocker
  graphics/2_glxgears_.*                     certification-status=blocker
  graphics/2_rotation_.*                     certification-status=blocker
@@ -75,37 +74,58 @@ _name: Graphics tests (discrete GPU) (Automated)
 _description:
  Graphics tests (discrete GPU) (Automated)
 include:
- graphics/2_valid_opengl_renderer_.*        certification-status=blocker
- graphics/2_driver_version_.*               certification-status=blocker
- graphics/2_compiz_check_.*                 certification-status=blocker
+ graphics/2_auto_switch_card_.*                  certification-status=blocker
+ graphics/2_valid_opengl_renderer_.*             certification-status=blocker
+ graphics/2_driver_version_.*                    certification-status=blocker
+ graphics/2_compiz_check_.*                      certification-status=blocker
  graphics/2_minimum_resolution_.*
+ suspend/2_resolution_before_suspend_.*_auto     certification-status=blocker
 bootstrap_include:
     graphics_card
 
-
 id: after-suspend-graphics-integrated-gpu-cert-full
 unit: test plan
 _name: After suspend tests (integrated GPU)
 _description: After suspend tests (integrated GPU)
 include:
-    graphics/1_auto_switch_card_.*                       certification-status=blocker
-    suspend/1_resolution_before_suspend_.*_auto          certification-status=blocker
+nested_part:
+ com.canonical.certification::after-suspend-graphics-integrated-gpu-cert-automated
+ com.canonical.certification::after-suspend-graphics-integrated-gpu-cert-manual
+
+id: after-suspend-graphics-integrated-gpu-cert-automated
+unit: test plan
+_name: After suspend tests (integrated GPU automated)
+_description: After suspend tests (integrated GPU automated)
+include:
+    graphics/1_auto_switch_card_.*                           certification-status=blocker
+    suspend/1_resolution_before_suspend_.*_auto              certification-status=blocker
+    suspend/1_suspend_after_switch_to_card_.*_auto           certification-status=blocker
     # The following after suspend jobs will automatically select the right suspend job
     # depending on the amount of graphic cards available on the SUT:
     # suspend/suspend_advanced (one GPU)
     # or suspend/{{ index }}_suspend_after_switch_to_card_{{ product_slug }}_auto (two GPUs)
-    suspend/1_suspend-time-check_.*_auto                 certification-status=non-blocker
-    suspend/1_suspend-single-log-attach_.*_auto
-    power-management/lid                                 certification-status=blocker
-    power-management/lid_close                           certification-status=blocker
-    power-management/lid_open                            certification-status=blocker
-    suspend/1_compiz_check_after_suspend_.*_auto         certification-status=blocker
-    suspend/1_driver_version_after_suspend_.*_auto       certification-status=blocker
-    suspend/1_resolution_after_suspend_.*_auto           certification-status=blocker
-    suspend/1_display_after_suspend_.*_auto              certification-status=blocker
-    suspend/1_glxgears_after_suspend_.*_auto             certification-status=blocker
-    suspend/1_video_after_suspend_.*_auto                certification-status=blocker
-    suspend/1_cycle_resolutions_after_suspend_.*_auto    certification-status=non-blocker
+    suspend/1_suspend-time-check_.*_auto                     certification-status=non-blocker
+    suspend/1_suspend-single-log-attach_.*_auto              certification-status=non-blocker
+    suspend/1_compiz_check_after_suspend_.*_auto             certification-status=blocker
+    suspend/1_driver_version_after_suspend_.*_auto           certification-status=blocker
+    suspend/1_resolution_after_suspend_.*_auto               certification-status=blocker
+
+id: after-suspend-graphics-integrated-gpu-cert-manual
+unit: test plan
+_name: After suspend tests (integrated GPU manual)
+_description: After suspend tests (integrated GPU manual)
+include:
+    # The following after suspend jobs will automatically select the right suspend job
+    # depending on the amount of graphic cards available on the SUT:
+    # suspend/suspend_advanced (one GPU)
+    # or suspend/{{ index }}_suspend_after_switch_to_card_{{ product_slug }}_auto (two GPUs)
+    power-management/lid                                     certification-status=blocker
+    power-management/lid_close                               certification-status=blocker
+    power-management/lid_open                                certification-status=blocker
+    suspend/1_display_after_suspend_.*_graphics              certification-status=blocker
+    suspend/1_glxgears_after_suspend_.*_graphics             certification-status=blocker
+    suspend/1_video_after_suspend_.*_graphics                certification-status=blocker
+    suspend/1_cycle_resolutions_after_suspend_.*_graphics    certification-status=non-blocker
     suspend/1_xrandr_screens_after_suspend.tar.gz_auto
 
 id: after-suspend-graphics-discrete-gpu-cert-full
@@ -113,19 +133,32 @@ unit: test plan
 _name: After suspend tests (discrete GPU)
 _description: After suspend tests (discrete GPU)
 include:
-    suspend/2_resolution_before_suspend_.*_auto          certification-status=blocker
-    suspend/2_suspend_after_switch_to_card_.*_auto       certification-status=blocker
-    suspend/2_suspend-time-check_.*_auto                 certification-status=non-blocker
-    suspend/2_suspend-single-log-attach_.*_auto
-    suspend/2_compiz_check_after_suspend_.*_auto         certification-status=blocker
-    suspend/2_driver_version_after_suspend_.*_auto       certification-status=blocker
-    suspend/2_resolution_after_suspend_.*_auto           certification-status=blocker
-    suspend/2_display_after_suspend_.*_auto              certification-status=blocker
-    suspend/2_glxgears_after_suspend_.*_auto             certification-status=blocker
-    suspend/2_video_after_suspend_.*_auto                certification-status=blocker
-    suspend/2_cycle_resolutions_after_suspend_.*_auto    certification-status=non-blocker
+nested_part:
+    com.canonical.certification::after-suspend-graphics-discrete-gpu-cert-automated
+    com.canonical.certification::after-suspend-graphics-discrete-gpu-cert-manual
+
+id: after-suspend-graphics-discrete-gpu-cert-automated
+unit: test plan
+_name: After suspend tests (discrete GPU automated)
+_description: After suspend tests (discrete GPU automated)
+include:
+    suspend/2_suspend_after_switch_to_card_.*_auto           certification-status=blocker
+    suspend/2_suspend-time-check_.*_auto                     certification-status=non-blocker
+    suspend/2_suspend-single-log-attach_.*_auto              certification-status=non-blocker
+    suspend/2_compiz_check_after_suspend_.*_auto             certification-status=blocker
+    suspend/2_driver_version_after_suspend_.*_auto           certification-status=blocker
+    suspend/2_resolution_after_suspend_.*_auto               certification-status=blocker
+
+id: after-suspend-graphics-discrete-gpu-cert-manual
+unit: test plan
+_name: After suspend tests (discrete GPU manual)
+_description: After suspend tests (discrete GPU manual)
+include:
+    suspend/2_display_after_suspend_.*_graphics              certification-status=blocker
+    suspend/2_glxgears_after_suspend_.*_graphics             certification-status=blocker
+    suspend/2_video_after_suspend_.*_graphics                certification-status=blocker
+    suspend/2_cycle_resolutions_after_suspend_.*_graphics    certification-status=non-blocker
     suspend/2_xrandr_screens_after_suspend_.*.tar.gz_auto
-    after-suspend-manual-monitor/2_dim_brightness_.*     certification-status=blocker
 
 id: graphics-integrated-gpu-cert-blockers
 unit: test plan
@@ -175,10 +208,9 @@ include:
     suspend/1_compiz_check_after_suspend_.*_auto           certification-status=blocker
     suspend/1_driver_version_after_suspend_.*_auto         certification-status=blocker
     suspend/1_resolution_after_suspend_.*_auto             certification-status=blocker
-    suspend/1_display_after_suspend_.*_auto                certification-status=blocker
-    suspend/1_glxgears_after_suspend_.*_auto               certification-status=blocker
-    suspend/1_video_after_suspend_.*_auto                  certification-status=blocker
-    after-suspend-manual-monitor/1_dim_brightness_.*       certification-status=blocker
+    suspend/1_display_after_suspend_.*_graphics            certification-status=blocker
+    suspend/1_glxgears_after_suspend_.*_graphics           certification-status=blocker
+    suspend/1_video_after_suspend_.*_graphics              certification-status=blocker
 
 id: after-suspend-graphics-discrete-gpu-cert-blockers
 unit: test plan
@@ -186,11 +218,10 @@ _name: After suspend tests (discrete GPU, certification blockers only)
 _description: After suspend tests (discrete GPU, certification blockers only)
 include:
     suspend/2_resolution_before_suspend_.*_auto          certification-status=blocker
-    suspend/2_suspend_after_switch_to_card_.*_auto       certification-status=blocker
+    suspend/2_suspend_after_switch_to_card_.*_graphics   certification-status=blocker
     suspend/2_compiz_check_after_suspend_.*_auto         certification-status=blocker
     suspend/2_driver_version_after_suspend_.*_auto       certification-status=blocker
     suspend/2_resolution_after_suspend_.*_auto           certification-status=blocker
-    suspend/2_display_after_suspend_.*_auto              certification-status=blocker
-    suspend/2_glxgears_after_suspend_.*_auto             certification-status=blocker
-    suspend/2_video_after_suspend_.*_auto                certification-status=blocker
-    after-suspend-manual-monitor/2_dim_brightness_.*     certification-status=blocker
+    suspend/2_display_after_suspend_.*_graphics          certification-status=blocker
+    suspend/2_glxgears_after_suspend_.*_graphics         certification-status=blocker
+    suspend/2_video_after_suspend_.*_graphics            certification-status=blocker
diff --git a/units/suspend/suspend-graphics.pxu b/units/suspend/suspend-graphics.pxu
index be820a7..d4fe923 100644
--- a/units/suspend/suspend-graphics.pxu
+++ b/units/suspend/suspend-graphics.pxu
@@ -14,7 +14,7 @@ command:
 unit: template
 template-resource: graphics_card
 template-filter: graphics_card.prime_gpu_offload == 'Off'
-plugin: user-interact-verify
+plugin: shell
 category_id: com.canonical.plainbox::suspend
 id: suspend/{index}_suspend_after_switch_to_card_{product_slug}_auto
 requires:
@@ -24,27 +24,12 @@ after: graphics/{index}_auto_switch_card_{product_slug}
 user: root
 environ: PLAINBOX_SESSION_SHARE
 command:
- if type -P fwts >/dev/null; then
-  echo "Calling fwts"
-  set -o pipefail; checkbox-support-fwts_test -f none -l $PLAINBOX_SESSION_SHARE/{index}_suspend_single -s s3 --s3-sleep-delay=30 --s3-device-check --s3-device-check-delay=45 | tee $PLAINBOX_SESSION_SHARE/{index}_suspend_single_times.log
- else
-  echo "Calling sleep_test"
-  set -o pipefail; sleep_test -p | tee $PLAINBOX_SESSION_SHARE/{index}_suspend_single_times.log
+ if [[ -v SNAP ]]; then
+     export LD_LIBRARY_PATH="$LD_LIBRARY_PATH:$SNAP/usr/lib/fwts"
  fi
-estimated_duration: 90.0
-_summary: Test suspend/resume after switching to {vendor} {product}
-_description:
- PURPOSE:
-     This test will check suspend and resume after switching to {vendor} {product} graphics card.
- STEPS:
-     1. Ensure you have switched to {vendor} {product} graphics card.
-     2. Click "Test" and your system will suspend for about 30 - 60 seconds
-     3. Observe the Power LED to see if it blinks or changes color during suspend
-     4. If your system does not wake itself up after 60 seconds, please press the power button momentarily to wake the system manually
-     5. If your system fails to wake at all and must be rebooted, restart System Testing after reboot and mark this test as Failed
- VERIFICATION:
-     Did your system suspend and resume correctly after switching to {vendor} {product} graphics card?
-     (NOTE: Please only consider whether the system successfully suspended and resumed. Power/Suspend LED verification will occur after this test is completed.)
+ set -o pipefail; checkbox-support-fwts_test -f none -l $PLAINBOX_SESSION_SHARE/{index}_suspend_single -s s3 --s3-sleep-delay=30 --s3-device-check --s3-device-check-delay=45 | tee $PLAINBOX_SESSION_SHARE/{index}_suspend_single_times.log
+estimated_duration: 1m30s
+_summary: Test auto suspend/resume after switching to {vendor} {product}
 
 unit: template
 template-resource: graphics_card
@@ -72,7 +57,7 @@ template-filter: graphics_card.prime_gpu_offload == 'Off'
 template-engine: jinja2
 plugin: manual
 category_id: com.canonical.plainbox::suspend
-id: suspend/{{ index }}_display_after_suspend_{{ product_slug }}_auto
+id: suspend/{{ index }}_display_after_suspend_{{ product_slug }}_graphics
 depends:
  {%- if gpu_count > "1" %}
   suspend/{{ index }}_suspend_after_switch_to_card_{{ product_slug }}_auto
@@ -94,7 +79,7 @@ template-filter: graphics_card.prime_gpu_offload == 'Off'
 template-engine: jinja2
 plugin: user-interact-verify
 category_id: com.canonical.plainbox::suspend
-id: suspend/{{ index }}_cycle_resolutions_after_suspend_{{ product_slug }}_auto
+id: suspend/{{ index }}_cycle_resolutions_after_suspend_{{ product_slug }}_graphics
 requires: package.name == 'xorg'
 depends:
  {%- if gpu_count > "1" %}
@@ -120,7 +105,7 @@ template-filter: graphics_card.prime_gpu_offload == 'Off'
 plugin: attachment
 category_id: com.canonical.plainbox::suspend
 id: suspend/{index}_xrandr_screens_after_suspend.tar.gz_auto
-depends: suspend/{index}_cycle_resolutions_after_suspend_{product_slug}_auto
+depends: suspend/{index}_cycle_resolutions_after_suspend_{product_slug}_graphics
 command: [ -f $PLAINBOX_SESSION_SHARE/{index}_xrandr_screens_after_suspend.tgz ] && cat $PLAINBOX_SESSION_SHARE/{index}_xrandr_screens_after_suspend.tgz
 _description: This attaches screenshots from the suspend/cycle_resolutions_after_suspend test to the results submission.
 
@@ -150,7 +135,7 @@ template-resource: graphics_card
 template-engine: jinja2
 plugin: user-interact-verify
 category_id: com.canonical.plainbox::suspend
-id: suspend/{{ index }}_glxgears_after_suspend_{{ product_slug }}_auto
+id: suspend/{{ index }}_glxgears_after_suspend_{{ product_slug }}_graphics
 depends:
  {%- if gpu_count > "1" %}
   suspend/{{ index }}_suspend_after_switch_to_card_{{ product_slug }}_auto
@@ -176,7 +161,7 @@ _description:
 unit: template
 template-resource: graphics_card
 template-engine: jinja2
-id: suspend/{{ index }}_video_after_suspend_{{ product_slug }}_auto
+id: suspend/{{ index }}_video_after_suspend_{{ product_slug }}_graphics
 depends:
  {%- if gpu_count > "1" %}
   suspend/{{ index }}_suspend_after_switch_to_card_{{ product_slug }}_auto