Merge lp:~kissiel/checkbox/jinja-exporter into lp:checkbox

[Maciej Kisielewski]

This MR brings basic, generic jinja2 exporter. To create concrete exporter instantiate Jinja2SessionStateExporter with particular jinja template.

9f799ce plainbox:exporter: add jinja2 exporter
a3b690d plainbox:abc: add dump_from_session_manager method to exporter iface
33151eb plainbox:abc: remove get_session_data_subset from ISessionStateExporter iface

[Zygmunt Krynicki (zyga)]

12:11 <@zyga> kissiel: -1 on your merge request, don't change the ABC
interface on the first patch
12:11 <@zyga> kissiel: that's broken broken broken, stuff that uses it
as a mock spec will fail, and it's a bad idea in general
12:12 < kissiel> mm-kay
12:12 <@zyga> kissiel: even if it works I think it should be there
until we get rid of it
12:12 <@zyga> kissiel: from the core classes
12:12 <@zyga> kissiel: typically abc interfaces are the last to go
12:12 <@zyga> kissiel: +1 on the next patch but again, wrong ordering
12:13 <@zyga> kissiel: first add it to the places that want it
12:13 <@zyga> kissiel: then declare it as interface
12:13 <@zyga> kissiel: otherwise between those commits, stuff is broken
12:13 <@zyga> kissiel: what I want to see
12:13 <@zyga> kissiel: add the new method to the base exporter class
12:13 <@zyga> kissiel: switch all tools to use the new method
12:13 <@zyga> kissiel: @deprecate the old method
12:13 <@zyga> kissiel: add the interface
12:13 <@zyga> kissiel: see if per commit stuff works
12:14 <@zyga> kissiel: add jinja stuff
12:14 <@zyga> kissiel: logical progression
12:14 <@zyga> kissiel: in the jinja code, use new-form super(), we're
not likely going to support 2.7 so no need to be verbose
12:15 <@zyga> kissiel: document the template argument, is that a
string or a filename?
12:15 <@zyga> kissiel: and one more thing, it's broken today so it's
not your fault at all but document how the stream is supposed to be
set up (binary or text)
12:15 <@zyga> kissiel: it's essential to know and we should stick to it
12:15 < kissiel> zyga, one thing tho, 'switch all the tools' this is
long-ish, and it's not like this can be easily fixed
12:16 <@zyga> kissiel: when mocking use spec_set= please, on the
interface if one exists, on the implementation if there is none
12:16 < kissiel> zyga, instead of patching it it should be replaced,
so maybe deprecate it and move to new exporters?
12:16 <@zyga> kissiel: if this cannot be easily fixed then don't do
the new method at all
12:16 <@zyga> kissiel: I mean currently all the expoerer code does
subset -> dump
12:17 <@zyga> kissiel: if you change that to dump_session() then this is fine
12:17 <@zyga> kissiel: if not then this is pointless, we want to have
one code that works in all the apps, not two that work in some and not
all, (or am I missing something?)
12:17 <@zyga> kissiel: you can implement the new interface on the
current classes easily from what I see
12:18 < kissiel> zyga, if every client class uses -> get_subset() &&
dump() then yes
12:18 <@zyga> kissiel: but from what I see, you're not implementing
the subset interface on Jinjj2SSE so you cannot just start using it
now
12:18 <@zyga> kissiel: that's what they have to use as that's the only
common interface
12:18 <@zyga> kissiel: all exporters are the same, options, subset, dump

On Mon, Mar 30, 2015 at 12:17 PM, Maciej Kisielewski
<email address hidden> wrote:
> The proposal to merge lp:~kissiel/checkbox/jinja-exporter into lp:checkbox has been updated.
>
> Status: Needs review => Work in progress
>
> For more details, see:
> https://code.launchpad.net/~kissiel/checkbox/jinja-exporter/+merge/254553
> --
> You are su...

[Zygmunt Krynicki (zyga)]

Just one comment about options and I'll be happy.

[lp:~kissiel/checkbox/jinja-exporter]

3634. By Zygmunt Krynicki on 2015-03-31

"automatic merge of lp:~zyga/checkbox/fix-1438643/ by tarmac [r=sylvain-pineau][bug=1438643][author=zyga]"

[Sylvain Pineau (sylvain-pineau)]

+1, We'll use this base exporter A LOT now. thanks

review: Approve

[Zygmunt Krynicki (zyga)]

One more mistake, sorry

[lp:~kissiel/checkbox/jinja-exporter]

3635. By Maciej Kisielewski on 2015-03-31

"automatic merge of lp:~kissiel/checkbox/jinja-exporter/ by tarmac [r=sylvain-pineau][bug=][author=kissiel]"

=== modified file 'checkbox-ng/checkbox_ng/commands/newcli.py'
--- checkbox-ng/checkbox_ng/commands/newcli.py	2015-03-05 16:20:40 +0000
+++ checkbox-ng/checkbox_ng/commands/newcli.py	2015-03-31 13:57:16 +0000
@@ -333,8 +333,7 @@
             print(self.C.header(_("Results")))
             exporter = get_all_exporters()['text']()
             exported_stream = io.BytesIO()
-            data_subset = exporter.get_session_data_subset(self.manager.state)
-            exporter.dump(data_subset, exported_stream)
+            exporter.dump_from_session_manager(self.manager, exported_stream)
             exported_stream.seek(0)  # Need to rewind the file, puagh
             # This requires a bit more finesse, as exporters output bytes
             # and stdout needs a string.
@@ -363,7 +362,6 @@
             actual_options = [opt for opt in exp_options
                               if opt in exporter_cls.supported_option_list]
             exporter = exporter_cls(actual_options)
-            data_subset = exporter.get_session_data_subset(self.manager.state)
             results_path = results_file
             if exporter_cls is XMLSessionStateExporter:
                 results_path = submission_file
@@ -372,7 +370,7 @@
                 if exporter_cls is XLSXSessionStateExporter:
                     results_path = results_path.replace('html', 'xlsx')
             with open(results_path, "wb") as stream:
-                exporter.dump(data_subset, stream)
+                exporter.dump_from_session_manager(self.manager, stream)
         print()
         print(_("Saving submission file to {}").format(submission_file))
         self.submission_file = submission_file

=== modified file 'checkbox-ng/checkbox_ng/commands/sru.py'
--- checkbox-ng/checkbox_ng/commands/sru.py	2014-11-24 10:57:17 +0000
+++ checkbox-ng/checkbox_ng/commands/sru.py	2015-03-31 13:57:16 +0000
@@ -116,10 +116,10 @@
 
     def _save_results(self):
         print(_("Saving results to {0}").format(self.config.fallback_file))
-        data = self.exporter.get_session_data_subset(self.session)
         with open(self.config.fallback_file, "wt", encoding="UTF-8") as stream:
             translating_stream = ByteStringStreamTranslator(stream, "UTF-8")
-            self.exporter.dump(data, translating_stream)
+            self.exporter.dump_from_session_manager(self.session.manager,
+                                                    translating_stream)
 
     def _submit_results(self):
         print(_("Submitting results to {0} for secure_id {1}").format(
@@ -128,10 +128,10 @@
         # Create the transport object
         transport = CertificationTransport(self.config.c3_url, options_string)
         # Prepare the data for submission
-        data = self.exporter.get_session_data_subset(self.session)
         with tempfile.NamedTemporaryFile(mode='w+b') as stream:
             # Dump the data to the temporary file
-            self.exporter.dump(data, stream)
+            self.exporter.dump_from_session_manager(self.session.manager,
+                                                    stream)
             # Flush and rewind
             stream.flush()
             stream.seek(0)

=== modified file 'checkbox-touch/py/checkbox_touch.py'
--- checkbox-touch/py/checkbox_touch.py	2015-03-20 13:15:08 +0000
+++ checkbox-touch/py/checkbox_touch.py	2015-03-31 13:57:16 +0000
@@ -728,9 +728,7 @@
                                   stream):
         exporter_cls = get_all_exporters()[output_format]
         exporter = exporter_cls(option_list)
-        data_subset = exporter.get_session_data_subset(
-            self.context.state)
-        exporter.dump(data_subset, stream)
+        export.dump_from_session_manager(self.manager, stream)
 
     def _checkpoint(self):
         self.context.state.metadata.app_blob = self._get_app_blob()

=== modified file 'plainbox/plainbox/abc.py'
--- plainbox/plainbox/abc.py	2015-02-09 09:08:25 +0000
+++ plainbox/plainbox/abc.py	2015-03-31 13:57:16 +0000
@@ -1009,7 +1009,7 @@
         """
 
     @abstractmethod
-    def get_session_data_subset(self, session):
+    def get_session_data_subset(self, session_manager):
         """
         Compute a subset of session data.
 
@@ -1038,6 +1038,15 @@
         """
         # TODO: Add a way for the stream to be binary as well.
 
+    @abstractmethod
+    def dump_from_session_manager(self, session_manager, stream):
+        """
+        Dump session information pulled from session manager to stream.
+
+        This method takes session manager instance, extracts session
+        information from it, and dumps it to a stream.
+        """
+
 
 class ISessionStateTransport(metaclass=ABCMeta):
     """

=== modified file 'plainbox/plainbox/impl/commands/inv_run.py'
--- plainbox/plainbox/impl/commands/inv_run.py	2015-02-25 23:54:36 +0000
+++ plainbox/plainbox/impl/commands/inv_run.py	2015-03-31 13:57:16 +0000
@@ -808,8 +808,7 @@
     def export_and_send_results(self):
         # Get a stream with exported session data.
         exported_stream = io.BytesIO()
-        data_subset = self.exporter.get_session_data_subset(self.state)
-        self.exporter.dump(data_subset, exported_stream)
+        self.exporter.dump_from_session_manager(self.manager, exported_stream)
         exported_stream.seek(0)  # Need to rewind the file, puagh
         # Write the stream to file if requested
         self._save_results(self.ns.output_file, exported_stream)

=== modified file 'plainbox/plainbox/impl/commands/inv_session.py'
--- plainbox/plainbox/impl/commands/inv_session.py	2015-02-06 15:51:40 +0000
+++ plainbox/plainbox/impl/commands/inv_session.py	2015-03-31 13:57:16 +0000
@@ -156,8 +156,7 @@
                 self._get_all_units(), storage, flags=self.ns.flag)
             # Get a stream with exported session data.
             exported_stream = io.BytesIO()
-            data_subset = exporter.get_session_data_subset(manager.state)
-            exporter.dump(data_subset, exported_stream)
+            exporter.dump_from_session_manager(manager, exported_stream)
             exported_stream.seek(0)  # Need to rewind the file, puagh
             # Write the stream to file if requested
             if self.ns.output_file is sys.stdout:

=== modified file 'plainbox/plainbox/impl/exporter/__init__.py'
--- plainbox/plainbox/impl/exporter/__init__.py	2015-02-13 14:58:46 +0000
+++ plainbox/plainbox/impl/exporter/__init__.py	2015-03-31 13:57:16 +0000
@@ -35,6 +35,7 @@
 
 from plainbox.i18n import gettext as _
 from plainbox.abc import ISessionStateExporter
+from plainbox.impl import deprecated
 
 logger = getLogger("plainbox.exporter")
 
@@ -161,7 +162,18 @@
         """
         return cls.SUPPORTED_OPTION_LIST
 
-    def get_session_data_subset(self, session):
+    def dump_from_session_manager(self, session_manager, stream):
+        """
+        Dump session information pulled from session manager to stream.
+
+        This method takes session manager instance, extracts session
+        information from it, and dumps it to a stream.
+        """
+
+        self.dump(self.get_session_data_subset(session_manager), stream)
+
+    @deprecated('0.21', 'use .dump_from_session_manager instead')
+    def get_session_data_subset(self, session_manager):
         """
         Compute a subset of session data.
 
@@ -175,6 +187,7 @@
         data = {
             'result_map': {}
         }
+        session = session_manager.state
         if self.OPTION_WITH_JOB_LIST in self._option_list:
             data['job_list'] = [job.id for job in session.job_list]
         if self.OPTION_WITH_RUN_LIST in self._option_list:

=== added file 'plainbox/plainbox/impl/exporter/jinja2.py'
--- plainbox/plainbox/impl/exporter/jinja2.py	1970-01-01 00:00:00 +0000
+++ plainbox/plainbox/impl/exporter/jinja2.py	2015-03-31 13:57:16 +0000
@@ -0,0 +1,84 @@
+# This file is part of Checkbox.
+#
+# Copyright 2015 Canonical Ltd.
+# Written by:
+#   Maciej Kisielewski <[e-mail]>
+#
+# Checkbox is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License version 3,
+# as published by the Free Software Foundation.
+#
+# Checkbox is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Checkbox.  If not, see <http://www.gnu.org/licenses/>.
+
+"""
+:mod:`plainbox.impl.exporter.jinja2` -- exporter using jinja2 templates
+=======================================================================
+
+.. warning::
+    THIS MODULE DOES NOT HAVE A STABLE PUBLIC API
+"""
+
+from jinja2 import Template
+
+from plainbox.abc import ISessionStateExporter
+
+
+class Jinja2SessionStateExporter(ISessionStateExporter):
+
+    """Session state exporter that renders output using jinja2 template."""
+
+    def __init__(self, option_list=None, jinja2_template=""):
+        """
+        Initialize a new Jinja2SessionStateExporter with given arguments.
+
+        :param option_list:
+            List of options that template might use to fine-tune rendering.
+        :param jinja2_template:
+            String with Jinja2 template that will be used.
+
+        """
+        self.option_list = option_list
+        self.template = Template(jinja2_template)
+
+    def dump(self, data, stream):
+        """
+        Render report using jinja2 and dump it to stream.
+
+        :param data:
+            Dict to be used when rendering template instance
+        :param stream:
+            Byte stream to write to.
+
+        """
+        stream.write(self.template.render(data).encode('UTF-8'))
+
+    def dump_from_session_manager(self, session_manager, stream):
+        """
+        Extract data from session_manager and dump it into the stream.
+
+        :param session_manager:
+            SessionManager instance that manages session to be exported by
+            this exporter
+        :param stream:
+            Byte stream to write to.
+
+        """
+        data = {
+            'manager': session_manager,
+            'options': self.option_list,
+        }
+        self.dump(data, stream)
+
+    def get_session_data_subset(self, session_manager):
+        """Compute a subset of session data."""
+        return {'manager': session_manager}
+
+    def supported_option_list(cls):
+        """ Return list of supported exporter options."""
+        return []

=== modified file 'plainbox/plainbox/impl/exporter/test_init.py'
--- plainbox/plainbox/impl/exporter/test_init.py	2015-02-13 14:58:46 +0000
+++ plainbox/plainbox/impl/exporter/test_init.py	2015-03-31 13:57:16 +0000
@@ -37,8 +37,10 @@
 from plainbox.impl.job import JobDefinition
 from plainbox.impl.result import MemoryJobResult, IOLogRecord
 from plainbox.impl.session import SessionState
+from plainbox.impl.session.manager import SessionManager
 from plainbox.impl.testing_utils import make_job, make_job_result
 from plainbox.impl.unit.category import CategoryUnit
+from plainbox.vendor import mock
 
 
 class ClassPropertyTests(TestCase):
@@ -129,8 +131,9 @@
     def test_defaults(self):
         # Test all defaults, with all options unset
         exporter = self.TestSessionStateExporter()
-        session = self.make_test_session()
-        data = exporter.get_session_data_subset(session)
+        session_manager = mock.Mock(spec_set=SessionManager,
+                                    state=self.make_test_session())
+        data = exporter.get_session_data_subset(session_manager)
         expected_data = {
             'result_map': {
                 'job_a': OrderedDict([
@@ -193,8 +196,10 @@
         with TemporaryDirectory() as scratch_dir:
             exporter = self.TestSessionStateExporter(
                 self.TestSessionStateExporter.supported_option_list)
-            session = self.make_realistic_test_session(scratch_dir)
-            data = exporter.get_session_data_subset(session)
+            session_manager = mock.Mock(
+                spec_set=SessionManager,
+                state=self.make_realistic_test_session(scratch_dir))
+            data = exporter.get_session_data_subset(session_manager)
         expected_data = {
             'job_list': ['job_a', 'job_b'],
             'run_list': ['job_b', 'job_a'],
@@ -295,7 +300,8 @@
         })
         # Create and export a session with the three units
         state = SessionState([cat_foo, cat_bar, job_froz])
-        data = exporter.get_session_data_subset(state)
+        session_manager = mock.Mock(spec_set=SessionManager, state=state)
+        data = exporter.get_session_data_subset(session_manager)
         # Ensure that only the foo category was used, and the bar category was
         # discarded as nothing was referencing it
         self.assertEqual(data['category_map'], {
@@ -317,7 +323,8 @@
         })
         # Create and export a session with that one job
         state = SessionState([job])
-        data = exporter.get_session_data_subset(state)
+        session_manager = mock.Mock(spec_set=SessionManager, state=state)
+        data = exporter.get_session_data_subset(session_manager)
         # Ensure that the special 'uncategorized' category is used
         self.assertEqual(data['category_map'], {
             '2013.com.canonical.plainbox::uncategorised': 'Uncategorised',

=== added file 'plainbox/plainbox/impl/exporter/test_jinja2.py'
--- plainbox/plainbox/impl/exporter/test_jinja2.py	1970-01-01 00:00:00 +0000
+++ plainbox/plainbox/impl/exporter/test_jinja2.py	2015-03-31 13:57:16 +0000
@@ -0,0 +1,75 @@
+# This file is part of Checkbox.
+#
+# Copyright 2015 Canonical Ltd.
+# Written by:
+#   Maciej Kisielewski <[e-mail]>
+#
+# Checkbox is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License version 3,
+# as published by the Free Software Foundation.
+#
+# Checkbox is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Checkbox.  If not, see <http://www.gnu.org/licenses/>.
+
+"""
+plainbox.impl.exporter.test_jinja2
+==================================
+
+Test definitions for plainbox.impl.exporter.jinja2 module
+"""
+
+from io import BytesIO
+from textwrap import dedent
+from unittest import TestCase
+
+from plainbox.impl.exporter.jinja2 import Jinja2SessionStateExporter
+from plainbox.impl.result import MemoryJobResult
+from plainbox.impl.unit.job import JobDefinition
+from plainbox.vendor import mock
+
+
+class Jinja2SessionStateExporterTests(TestCase):
+    def setUp(self):
+        self.prepare_manager_single_job()
+
+    def prepare_manager_single_job(self):
+        result = mock.Mock(spec_set=MemoryJobResult, outcome='fail',
+                           is_hollow=False)
+        result.tr_outcome.return_value = 'fail'
+        job = mock.Mock(spec_set=JobDefinition, id='job_id')
+        job.tr_summary.return_value = 'job name'
+        self.manager_single_job = mock.Mock(state=mock.Mock(
+            run_list=[job],
+            job_state_map={
+                job.id: mock.Mock(result=result, job=job)
+            })
+        )
+
+    def test_plaintext_template(self):
+        tmpl = dedent(
+            "{% for job in manager.state.job_state_map %}"
+            "{{'{:^15}: {}'.format("
+            "manager.state.job_state_map[job].result.tr_outcome(),"
+            "manager.state.job_state_map[job].job.tr_summary()) }}\n"
+            "{% endfor %}")
+
+        exporter = Jinja2SessionStateExporter(jinja2_template=tmpl)
+        stream = BytesIO()
+        exporter.dump_from_session_manager(self.manager_single_job, stream)
+        expected_bytes = '     fail      : job name\n'.encode('UTF-8')
+        self.assertEqual(stream.getvalue(), expected_bytes)
+
+    def test_empty_template(self):
+        """
+        Ensure that exporter doesn't print anything when jinja2 template is
+        explictly empty.
+        """
+        exporter = Jinja2SessionStateExporter(jinja2_template="")
+        stream = BytesIO()
+        exporter.dump_from_session_manager(self.manager_single_job, stream)
+        self.assertEqual(stream.getvalue(), b'')

=== modified file 'plainbox/plainbox/impl/exporter/text.py'
--- plainbox/plainbox/impl/exporter/text.py	2015-01-08 12:39:39 +0000
+++ plainbox/plainbox/impl/exporter/text.py	2015-03-31 13:57:16 +0000
@@ -39,8 +39,8 @@
         super().__init__(option_list)
         self.C = Colorizer(color)
 
-    def get_session_data_subset(self, session):
-        return session
+    def get_session_data_subset(self, session_manager):
+        return session_manager.state
 
     def dump(self, session, stream):
         for job in session.run_list:

=== modified file 'plainbox/plainbox/impl/highlevel.py'
--- plainbox/plainbox/impl/highlevel.py	2015-02-06 15:51:40 +0000
+++ plainbox/plainbox/impl/highlevel.py	2015-03-31 13:57:16 +0000
@@ -365,8 +365,7 @@
                                   stream):
         exporter_cls = get_all_exporters()[output_format]
         exporter = exporter_cls(option_list)
-        data_subset = exporter.get_session_data_subset(session)
-        exporter.dump(data_subset, stream)
+        exporter.dump_from_session_manager(session.manager, stream)
 
     def get_all_transports(self):
         return [transport for transport in get_all_transports()]