Merge ~sylvain-pineau/checkbox-ng:fix-1817029 into checkbox-ng:master

[Sylvain Pineau]

Fixes the linked bug.

This MR is mostly based on Pierre's https://code.launchpad.net/~pieq/checkbox-ng/+git/checkbox-ng/+merge/367634

The only difference is the category mapping used to render html reports (category_map_lite).

Bonus: a Fix to dkms_info_json to remove looging recorded on sdterr and messing up the json validation.

[Sylvain Pineau (sylvain-pineau)]

                result['id'] = result.get('full_id', result['id'])
                if "::" not in result['id']:
                    result['id'] = CERTIFICATION_NS + result['id']

The fallback to result['id'] actually makes thsi code compatible with old submissions (e.g the one linked in the bug report: https://certification.canonical.com/hardware/201812-26755/submission/138731/)

[Pierre Equoy (pieq)]

Tested the latest fix like so:

1. checkbox-cli, select 18.04 test plan, then a few jobs from it (e.g. optical/detect ...)
2. during the execution, mark one of the manual jobs as Failed
→ You get a submission (let's call it sub1.tar.xz)
3. re-launch checkbox-cli, select the same test plans and the same few jobs
4. during the execution, mark the job you marked as Failed in the previous run as Passed
→ You get a submission (let's call it sub2.tar.xz)
5. Let's try to merge one submission with itself to make sure we get the same results:

$ checkbox-cli merge-submissions -o /tmp/selfmerge.tar.xz sub1.tar.xz sub1.tar.xz

→ Both HTML and JSON files are the same in selfmerge.tar.xz and sub1.tar.xz (OK)

6. Now let's merge 2 different submissions to make sure the feature actually works as expected :)

$ checkbox-cli merge-submissions -o /tmp/selfmerge.tar.xz sub1.tar.xz sub2.tar.xz

→ Since I have selected exactly the same jobs for sub1 and sub2, and marked the job as Passed in sub2, selfmerge.tar.xz and sub2.tar.xz HTML and json files are identical (OK)

7. Let's merge the submissions, but in reverse order:

$ checkbox-cli merge-submissions -o /tmp/selfmerge.tar.xz sub2.tar.xz sub1.tar.xz

This time, selfmerge.tar.xz HTML and json files are similar to the ones in sub1.tar.xz (OK)

+1 to land this.

Thanks a lot Sylvain !

(I've noticed the submission title disappearance, but I think we'll live without it :))

review: Approve

diff --git a/checkbox_ng/launcher/merge_reports.py b/checkbox_ng/launcher/merge_reports.py
index 99fecaf..2eed566 100644
--- a/checkbox_ng/launcher/merge_reports.py
+++ b/checkbox_ng/launcher/merge_reports.py
@@ -37,6 +37,10 @@ from plainbox.impl.unit.category import CategoryUnit
 from plainbox.impl.unit.job import JobDefinition
 
 
+#: Name-space prefix for Canonical Certification
+CERTIFICATION_NS = 'com.canonical.certification::'
+
+
 class MergeReports(Command):
     name = 'merge-reports'
 
@@ -58,6 +62,10 @@ class MergeReports(Command):
             for result in data['results']:
                 result['plugin'] = 'shell'  # Required so default to shell
                 result['summary'] = result['name']
+                # 'id' field in json file only contains partial id
+                result['id'] = result.get('full_id', result['id'])
+                if "::" not in result['id']:
+                    result['id'] = CERTIFICATION_NS + result['id']
                 if mode == "list":
                     self.job_list.append(JobDefinition(result))
                 elif mode == "dict":
@@ -65,6 +73,10 @@ class MergeReports(Command):
             for result in data['resource-results']:
                 result['plugin'] = 'resource'
                 result['summary'] = result['name']
+                # 'id' field in json file only contains partial id
+                result['id'] = result.get('full_id', result['id'])
+                if "::" not in result['id']:
+                    result['id'] = CERTIFICATION_NS + result['id']
                 if mode == "list":
                     self.job_list.append(JobDefinition(result))
                 elif mode == "dict":
@@ -72,6 +84,10 @@ class MergeReports(Command):
             for result in data['attachment-results']:
                 result['plugin'] = 'attachment'
                 result['summary'] = result['name']
+                # 'id' field in json file only contains partial id
+                result['id'] = result.get('full_id', result['id'])
+                if "::" not in result['id']:
+                    result['id'] = CERTIFICATION_NS + result['id']
                 if mode == "list":
                     self.job_list.append(JobDefinition(result))
                 elif mode == "dict":
@@ -98,7 +114,8 @@ class MergeReports(Command):
                     keepends=True))
         ]
         result = MemoryJobResult({
-            'outcome': job.get_record_value('status'),
+            'outcome': job.get_record_value('outcome',
+                                            job.get_record_value('status')),
             'comments': job.get_record_value('comments'),
             'execution_duration': job.get_record_value('duration'),
             'io_log': io_log,
@@ -111,12 +128,12 @@ class MergeReports(Command):
                 new_resource_list.append(resource)
             if not new_resource_list:
                 new_resource_list = [Resource({})]
-            state.set_resource_list(
-                "com.canonical.certification::" + job.id,
-                new_resource_list)
+            state.set_resource_list(job.id, new_resource_list)
         job_state = state.job_state_map[job.id]
         job_state.effective_category_id = job.get_record_value(
             'category_id', 'com.canonical.plainbox::uncategorised')
+        job_state.effective_certification_status = job.get_record_value(
+            'certification_status', 'unspecified')
 
     def _create_exporter(self, exporter_id):
         exporter_map = {}
diff --git a/plainbox/impl/exporter/jinja2.py b/plainbox/impl/exporter/jinja2.py
index 59d7b3b..c6dcfc8 100644
--- a/plainbox/impl/exporter/jinja2.py
+++ b/plainbox/impl/exporter/jinja2.py
@@ -49,10 +49,8 @@ CERTIFICATION_NS = 'com.canonical.certification::'
 @environmentfilter
 def do_strip_ns(_environment, unit_id, ns=CERTIFICATION_NS):
     """Remove the namespace part of the identifier."""
-    if unit_id.startswith(ns):
-        rv = unit_id[len(ns):]
-    else:
-        rv = unit_id
+    # com.my.namespace::category/job-id → category/job-id
+    rv = unit_id.split("::")[-1]
     rv = escape(rv)
     if _environment.autoescape:
         rv = Markup(rv)
diff --git a/plainbox/impl/providers/exporters/data/checkbox.html b/plainbox/impl/providers/exporters/data/checkbox.html
index 88ecb47..9c80c81 100644
--- a/plainbox/impl/providers/exporters/data/checkbox.html
+++ b/plainbox/impl/providers/exporters/data/checkbox.html
@@ -2,7 +2,7 @@
 {%- set state = manager.default_device_context.state -%}
 {%- set resource_map = state.resource_map -%}
 {%- set job_state_map = state.job_state_map -%}
-{%- set category_map = state.category_map -%}
+{%- set category_map = state.category_map_lite -%}
 {%- set category_outcome_map = state.category_outcome_map -%}
 {%- set resource_global_outcome = state.resource_global_outcome -%}
 {%- set attachment_global_outcome = state.attachment_global_outcome -%}
@@ -177,7 +177,7 @@
                             <tbody>
                             {%- for job_id, job_state in job_state_map|dictsort if job_state.result.outcome != None and job_state.effective_category_id == cat_id and job_state.job.plugin not in ("resource", "attachment") %}
                                 <tr>
-                                    <td data-filtertext="{{ job_state.job.partial_id }}" style='width:35%'>{{ job_state.job.partial_id }}</td>
+                                    <td data-filtertext="{{ job_id|strip_ns }}" style='width:35%'>{{ job_id|strip_ns }}</td>
                                     <td style='width:10%; font-weight: bold; color: {{ job_state.result.outcome_meta().color_hex }}'>{{ job_state.result.outcome_meta().tr_label }}</td>
                                     {%- if job_state.effective_certification_status != "unspecified" %}
                                     <td style='width:10%'>{{ job_state.effective_certification_status }}</td>
@@ -208,9 +208,9 @@
                             </thead>
                             <tbody>
                             {%- for job_id, job_state in job_state_map|dictsort if job_state.result.outcome != None and job_state.job.plugin == "resource" %}
-                                {%- if job_state.job.partial_id == "package" %}
+                                {%- if job_id|strip_ns == "package" %}
                                 <tr>
-                                    <td data-filtertext="{{ job_state.job.partial_id }}" style='width:35%'>{{ job_state.job.partial_id }}</td>
+                                    <td data-filtertext="{{ job_id|strip_ns }}" style='width:35%'>{{ job_id|strip_ns }}</td>
                                     <td style='width:10%; font-weight: bold; color: {{ job_state.result.outcome_meta().color_hex }}'>{{ job_state.result.outcome_meta().tr_label }}</td>
                                     {%- if job_state.effective_certification_status != "unspecified" %}
                                     <td style='width:10%'>{{ job_state.effective_certification_status }}</td>
@@ -226,7 +226,7 @@
                                 </tr>
                                 {%- else %}
                                 <tr>
-                                    <td data-filtertext="{{ job_state.job.partial_id }}" style='width:35%'>{{ job_state.job.partial_id }}</td>
+                                    <td data-filtertext="{{ job_id|strip_ns }}" style='width:35%'>{{ job_id|strip_ns }}</td>
                                     <td style='width:10%; font-weight: bold; color: {{ job_state.result.outcome_meta().color_hex }}'>{{ job_state.result.outcome_meta().tr_label }}</td>
                                     {%- if job_state.effective_certification_status != "unspecified" %}
                                     <td style='width:10%'>{{ job_state.effective_certification_status }}</td>
@@ -258,7 +258,7 @@
                             <tbody>
                     {%- endif %}
                                 <tr>
-                                    <td data-filtertext="{{ job_state.job.partial_id }}" style='width:35%'>{{ job_state.job.partial_id }}</td>
+                                    <td data-filtertext="{{ job_id|strip_ns }}" style='width:35%'>{{ job_id|strip_ns }}</td>
                                     <td style='width:10%; font-weight: bold; color: {{ job_state.result.outcome_meta().color_hex }}'>{{ job_state.result.outcome_meta().tr_label }}</td>
                                     {%- if job_state.effective_certification_status != "unspecified" %}
                                     <td style='width:10%'>{{ job_state.effective_certification_status }}</td>
@@ -329,7 +329,7 @@
         {%- endfor %}
     {%- endfor %}
     {%- for job_id, job_state in job_state_map|dictsort if job_state.result.outcome != None and job_state.job.plugin == "resource" %}
-    {%- if job_state.result.io_log_as_flat_text != "" and job_state.job.partial_id != "package" %}
+    {%- if job_state.result.io_log_as_flat_text != "" and job_id|strip_ns != "package" %}
     <div class="jqm-demos ui-page" tabindex="0" data-url="resource-{{ loop.index }}" id="resource-{{ loop.index }}-log" data-role="page">
         <div data-role="header" class="jqm-header">
             <h1>{{ job_state.job.tr_summary() }}</h1>
diff --git a/plainbox/impl/providers/exporters/data/checkbox.json b/plainbox/impl/providers/exporters/data/checkbox.json
index 7f40ade..3c8e204 100644
--- a/plainbox/impl/providers/exporters/data/checkbox.json
+++ b/plainbox/impl/providers/exporters/data/checkbox.json
@@ -46,11 +46,13 @@
     {%- for job_id, job_state in job_state_map|dictsort if job_state.result.outcome != None and job_state.job.plugin not in ("resource", "attachment") %}
         {
             "id": "{{ job_id|strip_ns }}",
+            "full_id": "{{ job_id }}",
             "name": "{{ job_state.job.tr_summary() }}",
             "certification_status": "{{ job_state.effective_certification_status }}",
             "category": "{{ category_map[job_state.effective_category_id] }}",
             "category_id": "{{ job_state.effective_category_id }}",
             "status": "{{ job_state.result.outcome_meta().hexr_mapping }}",
+            "outcome": "{{ job_state.result.outcome }}",
             "comments": {{ job_state.result.comments | jsonify | safe }},
             "io_log": {{ job_state.result.io_log_as_flat_text | jsonify | safe }},
             "type": "test",
@@ -63,10 +65,13 @@
     {%- for job_id, job_state in job_state_map|dictsort if job_state.result.outcome != None and job_state.job.plugin == "resource" %}
         {
             "id": "{{ job_id|strip_ns }}",
+            "full_id": "{{ job_id }}",
             "name": "{{ job_state.job.tr_summary() }}",
             "certification_status": "{{ job_state.effective_certification_status }}",
             "category": "{{ category_map[job_state.effective_category_id] }}",
+            "category_id": "{{ job_state.effective_category_id }}",
             "status": "{{ job_state.result.outcome_meta().hexr_mapping }}",
+            "outcome": "{{ job_state.result.outcome }}",
             "comments": {{ job_state.result.comments | jsonify | safe }},
             "io_log": {{ job_state.result.io_log_as_flat_text | jsonify | safe }},
             "type": "test",
@@ -79,10 +84,13 @@
     {%- for job_id, job_state in job_state_map|dictsort if job_state.result.outcome != None and job_state.job.plugin == "attachment" %}
         {
             "id": "{{ job_id|strip_ns }}",
+            "full_id": "{{ job_id }}",
             "name": "{{ job_state.job.tr_summary() }}",
             "certification_status": "{{ job_state.effective_certification_status }}",
             "category": "{{ category_map[job_state.effective_category_id] }}",
+            "category_id": "{{ job_state.effective_category_id }}",
             "status": "{{ job_state.result.outcome_meta().hexr_mapping }}",
+            "outcome": "{{ job_state.result.outcome }}",
             "comments": {{ job_state.result.comments | jsonify | safe }},
             "io_log": {{ job_state.result.io_log_as_text_attachment | jsonify | safe }},
             "duration": {{ job_state.result.execution_duration if job_state.result.execution_duration else 0}}
@@ -95,7 +103,7 @@
         {%- endfor %}
     }
 {%- if ns ~ 'dkms_info_json' in state.job_state_map and state.job_state_map[ns ~ 'dkms_info_json'].result.outcome == 'pass' %},
-{%- set dkms_info_json = state.job_state_map[ns ~ 'dkms_info_json'].result.io_log_as_text_attachment %}
+{%- set dkms_info_json = '{' + state.job_state_map[ns ~ 'dkms_info_json'].result.io_log_as_text_attachment.split('{', 1)[-1] %}
     "dkms_info": {{ dkms_info_json | indent(4, false) | safe }}
 {%- endif %}
 {%- if ns ~ 'udev_json' in state.job_state_map and state.job_state_map[ns ~ 'udev_json'].result.outcome == 'pass' %},
diff --git a/plainbox/impl/providers/exporters/data/multi-page.html b/plainbox/impl/providers/exporters/data/multi-page.html
index 6e511b4..aee86ec 100644
--- a/plainbox/impl/providers/exporters/data/multi-page.html
+++ b/plainbox/impl/providers/exporters/data/multi-page.html
@@ -65,10 +65,11 @@
 {% for manager in manager_list %}
 {% set managerloop = loop %}
 {%- set metadata = manager.state.metadata %}
+{%- set ns = 'com.canonical.certification::' -%}
 {%- set state = manager.default_device_context.state -%}
 {%- set resource_map = state.resource_map -%}
 {%- set job_state_map = state.job_state_map -%}
-{%- set category_map = state.category_map -%}
+{%- set category_map = state.category_map_lite -%}
 {%- set category_outcome_map = state.category_outcome_map -%}
 {%- set resource_global_outcome = state.resource_global_outcome -%}
 {%- set attachment_global_outcome = state.attachment_global_outcome -%}
@@ -224,7 +225,7 @@
             <tbody>
             {%- for job_id, job_state in job_state_map|dictsort if job_state.result.outcome != None and job_state.effective_category_id == cat_id and job_state.job.plugin not in ("resource", "attachment") %}
                 <tr>
-                    <td data-filtertext="{{ job_state.job.partial_id }}" style='width:35%'>{{ job_state.job.partial_id }}</td>
+                    <td data-filtertext="{{ job_id|strip_ns }}" style='width:35%'>{{ job_id|strip_ns }}</td>
                     <td style='width:10%; font-weight: bold; color: {{ job_state.result.outcome_meta().color_hex }}'>{{ job_state.result.outcome_meta().tr_label }}</td>
                     {%- if job_state.effective_certification_status != "unspecified" %}
                     <td style='width:10%'>{{ job_state.effective_certification_status }}</td>
@@ -255,9 +256,9 @@
             </thead>
             <tbody>
             {%- for job_id, job_state in job_state_map|dictsort if job_state.result.outcome != None and job_state.job.plugin == "resource" %}
-                {%- if job_state.job.partial_id == "package" %}
+                {%- if job_id|strip_ns == "package" %}
                 <tr>
-                    <td data-filtertext="{{ job_state.job.partial_id }}" style='width:35%'>{{ job_state.job.partial_id }}</td>
+                    <td data-filtertext="{{ job_id|strip_ns }}" style='width:35%'>{{ job_id|strip_ns }}</td>
                     <td style='width:10%; font-weight: bold; color: {{ job_state.result.outcome_meta().color_hex }}'>{{ job_state.result.outcome_meta().tr_label }}</td>
                     {%- if job_state.effective_certification_status != "unspecified" %}
                     <td style='width:10%'>{{ job_state.effective_certification_status }}</td>
@@ -273,7 +274,7 @@
                 </tr>
                 {%- else %}
                 <tr>
-                    <td data-filtertext="{{ job_state.job.partial_id }}" style='width:35%'>{{ job_state.job.partial_id }}</td>
+                    <td data-filtertext="{{ job_id|strip_ns }}" style='width:35%'>{{ job_id|strip_ns }}</td>
                     <td style='width:10%; font-weight: bold; color: {{ job_state.result.outcome_meta().color_hex }}'>{{ job_state.result.outcome_meta().tr_label }}</td>
                     {%- if job_state.effective_certification_status != "unspecified" %}
                     <td style='width:10%'>{{ job_state.effective_certification_status }}</td>
@@ -305,7 +306,7 @@
             <tbody>
     {%- endif %}
                 <tr>
-                    <td data-filtertext="{{ job_state.job.partial_id }}" style='width:35%'>{{ job_state.job.partial_id }}</td>
+                    <td data-filtertext="{{ job_id|strip_ns }}" style='width:35%'>{{ job_id|strip_ns }}</td>
                     <td style='width:10%; font-weight: bold; color: {{ job_state.result.outcome_meta().color_hex }}'>{{ job_state.result.outcome_meta().tr_label }}</td>
                     {%- if job_state.effective_certification_status != "unspecified" %}
                     <td style='width:10%'>{{ job_state.effective_certification_status }}</td>
@@ -386,7 +387,7 @@
 <!-- Resources I/O log pages -->
 
 {%- for job_id, job_state in job_state_map|dictsort if job_state.result.outcome != None and job_state.job.plugin == "resource" %}
-{%- if job_state.result.io_log_as_flat_text != "" and job_state.job.partial_id != "package" %}
+{%- if job_state.result.io_log_as_flat_text != "" and job_id|strip_ns != "package" %}
 <div class="jqm-demos ui-page" tabindex="0" data-url="resource-{{ managerloop.index }}-{{ loop.index }}" id="resource-{{ managerloop.index }}-{{ loop.index }}-log" data-role="page">
     <div data-role="header" class="jqm-header">
         <h1>{{ job_state.job.tr_summary() }}</h1>
diff --git a/plainbox/impl/session/state.py b/plainbox/impl/session/state.py
index bd7bdef..833bbb8 100644
--- a/plainbox/impl/session/state.py
+++ b/plainbox/impl/session/state.py
@@ -1246,6 +1246,27 @@ class SessionState:
         wanted_category_ids = frozenset({
                 job_state.effective_category_id
                 for job_state in self.job_state_map.values()
+                if job_state.result.outcome != None
+            })
+        return {
+            unit.id: unit.tr_name()
+            for unit in self.unit_list
+            if unit.Meta.name == 'category'
+            and unit.id in wanted_category_ids
+        }
+
+    @property
+    def category_map_lite(self):
+        """
+        Map from category id to their corresponding translated names but
+        without taking into account resources and attachments jobs.
+
+        Meant to be used to generate the HTML reports where resources and
+        attachments are presented in different sections.
+        """
+        wanted_category_ids = frozenset({
+                job_state.effective_category_id
+                for job_state in self.job_state_map.values()
                 if job_state.result.outcome != None and
                 job_state.job.plugin not in ("resource", "attachment")
             })