[Merge] lp:~juliank/update-notifier/esm into lp:update-notifier

Brian Murray brian at ubuntu.com
Mon Apr 1 18:14:13 UTC 2019


A few more comments, thanks for addressing the other ones.

Diff comments:

> === modified file 'data/apt_check.py'
> --- data/apt_check.py	2018-11-13 20:53:28 +0000
> +++ data/apt_check.py	2019-04-01 15:02:31 +0000
> @@ -61,19 +70,77 @@
>      outstream.write("\n".join([p.name for p in pkgs]))
>  
>  
> -def write_human_readable_summary(outstream, upgrades, security_updates):
> +def write_human_readable_summary(outstream, upgrades, security_updates,
> +                                 esm_updates, have_esm, disabled_esm_updates):
>      " write out human summary summary to outstream "
> +    if have_esm is not None:
> +        if have_esm:

Don't these two checks do the same thing?

> +            outstream.write(gettext.dgettext("update-notifier",
> +                                             "Extended Security Maintenance "
> +                                             "(ESM) is enabled."))
> +        else:
> +            outstream.write(gettext.dgettext("update-notifier",
> +                                             "Extended Security Maintenance "
> +                                             "(ESM) is not enabled."))
> +        outstream.write("\n\n")
> +
>      outstream.write(gettext.dngettext("update-notifier",
> -                                      "%i package can be updated.",
> -                                      "%i packages can be updated.",
> +                                      "%i update can be installed "
> +                                      "immediately.",
> +                                      "%i updates can be installed "
> +                                      "immediately.",
>                                        upgrades) % upgrades)
>      outstream.write("\n")
> +    if esm_updates > 0:
> +        outstream.write(gettext.dngettext("update-notifier",
> +                                          "%i of these updates is "
> +                                          "provided through ESM.",
> +                                          "%i of these updates are "
> +                                          "provided through ESM.",
> +                                          esm_updates) %
> +                        esm_updates)
> +        outstream.write("\n")
>      outstream.write(gettext.dngettext("update-notifier",
> -                                      "%i update is a security update.",
> -                                      "%i updates are security updates.",
> -                                      security_updates) % security_updates)
> +                                      "%i of these updates is a "
> +                                      "security update.",
> +                                      "%i of these updates are "
> +                                      "security updates.",
> +                                      security_updates) %
> +                    security_updates)
>      outstream.write("\n")
>  
> +    if disabled_esm_updates > 0:
> +        outstream.write("\n")
> +        outstream.write(gettext.dngettext("update-notifier",
> +                                          "Enable ESM to get %i additional "
> +                                          "security update.",
> +                                          "Enable ESM to get %i additional "
> +                                          "security updates.",
> +                                          disabled_esm_updates) %
> +                        disabled_esm_updates)
> +        outstream.write("\n")
> +        outstream.write(gettext.dgettext("update-notifier",
> +                                         "See 'ua enable esm' or "
> +                                         "https://ubuntu.com/esm"))
> +        outstream.write("\n")
> +
> +
> +def has_disabled_esm_security_update(depcache, pkg):
> +    " check if we have a disabled ESM security update "
> +    inst_ver = pkg.current_ver
> +    if not inst_ver:
> +        return False
> +
> +    for ver in pkg.version_list:
> +        if ver == inst_ver:
> +            break
> +
> +        for (file, index) in ver.file_list:
> +            if (file.origin == "UbuntuESM" and file.archive.startswith(DISTRO)
> +                    and depcache.policy.get_priority(file) == -32768):
> +                return True
> +    return False
> +
>  
>  def init():
>      " init the system, be nice "
> 
> === added symlink 'tests/apt_check.py'
> === target is u'../data/apt_check.py'
> === added file 'tests/test_motd.py'
> --- tests/test_motd.py	1970-01-01 00:00:00 +0000
> +++ tests/test_motd.py	2019-04-01 15:02:31 +0000
> @@ -0,0 +1,137 @@
> +#!/usr/bin/python3
> +# -*- Mode: Python; indent-tabs-mode: nil; tab-width: 4; coding: utf-8 -*-
> +
> +import apt_check
> +import io
> +import os
> +import subprocess
> +import unittest
> +import textwrap
> +
> +
> +def get_message(*args, **kwds):
> +    with io.StringIO() as stream:
> +        apt_check.write_human_readable_summary(stream, *args, **kwds)
> +        return stream.getvalue()
> +
> +
> +class TestMotd(unittest.TestCase):
> +    """ ensure that the tree is pep8 clean """
> +
> +    def test_esm_disabled_upto_date_esm_avail(self):
> +        self.assertEqual(
> +            get_message(upgrades=0, security_updates=0,
> +                        esm_updates=0, have_esm=False,
> +                        disabled_esm_updates=23),
> +            textwrap.dedent(
> +                """
> +                Extended Security Maintenance (ESM) is not enabled.
> +
> +                0 updates can be installed immediately.
> +                0 of these updates are security updates.
> +
> +                Enable ESM to get 23 additional security updates.

I realize the document says "to get" but I think "to receive" is more professional.

> +                See 'ua enable esm' or https://ubuntu.com/esm
> +                """).lstrip())
> +
> +    def test_esm_disabled_security_esm_avail(self):
> +        self.assertEqual(
> +            get_message(upgrades=15, security_updates=7,
> +                        esm_updates=0, have_esm=False,
> +                        disabled_esm_updates=23),
> +            textwrap.dedent(
> +                """
> +                Extended Security Maintenance (ESM) is not enabled.
> +
> +                15 updates can be installed immediately.
> +                7 of these updates are security updates.
> +
> +                Enable ESM to get 23 additional security updates.
> +                See 'ua enable esm' or https://ubuntu.com/esm
> +                """).lstrip())
> +
> +    def test_esm_disabled_security_no_esm_avail(self):
> +        self.assertEqual(
> +            get_message(upgrades=15, security_updates=7,
> +                        esm_updates=0, have_esm=False,
> +                        disabled_esm_updates=0),
> +            textwrap.dedent(
> +                """
> +                Extended Security Maintenance (ESM) is not enabled.
> +
> +                15 updates can be installed immediately.
> +                7 of these updates are security updates.
> +                """).lstrip())
> +
> +    def test_esm_disabled_nosecurity(self):
> +        self.assertEqual(
> +            get_message(upgrades=15, security_updates=0,
> +                        esm_updates=0, have_esm=False,
> +                        disabled_esm_updates=0),
> +            textwrap.dedent(
> +                """
> +                Extended Security Maintenance (ESM) is not enabled.
> +
> +                15 updates can be installed immediately.
> +                0 of these updates are security updates.
> +                """).lstrip())
> +
> +    def test_esm_disabled_noupdates(self):
> +        self.assertEqual(
> +            get_message(upgrades=0, security_updates=0,
> +                        esm_updates=0, have_esm=False,
> +                        disabled_esm_updates=0),
> +            textwrap.dedent(
> +                """
> +                Extended Security Maintenance (ESM) is not enabled.
> +
> +                0 updates can be installed immediately.
> +                0 of these updates are security updates.
> +                """).lstrip())
> +
> +    def test_esm_enabled_nosecurity(self):
> +        self.assertEqual(
> +            get_message(upgrades=35, security_updates=0,
> +                        esm_updates=13, have_esm=True,
> +                        disabled_esm_updates=0),
> +            textwrap.dedent(
> +                """
> +                Extended Security Maintenance (ESM) is enabled.
> +
> +                35 updates can be installed immediately.
> +                13 of these updates are provided through ESM.
> +                0 of these updates are security updates.
> +                """).lstrip())
> +
> +    def test_esm_enabled_somesecurity(self):
> +        self.assertEqual(
> +            get_message(upgrades=47, security_updates=7,
> +                        esm_updates=13, have_esm=True,
> +                        disabled_esm_updates=0),
> +            textwrap.dedent(
> +                """
> +                Extended Security Maintenance (ESM) is enabled.
> +
> +                47 updates can be installed immediately.
> +                13 of these updates are provided through ESM.
> +                7 of these updates are security updates.
> +                """).lstrip())
> +
> +    def test_esm_enabled_noupdates(self):
> +        self.assertEqual(
> +            get_message(upgrades=0, security_updates=0,
> +                        esm_updates=0, have_esm=True,
> +                        disabled_esm_updates=0),
> +            textwrap.dedent(
> +                """
> +                Extended Security Maintenance (ESM) is enabled.
> +
> +                0 updates can be installed immediately.
> +                0 of these updates are security updates.

Indicating that 0 of the 0 updates are security related is redundant and takes up an extra line on the motd. Could that be dropped?

> +                """).lstrip())
> +
> +
> +if __name__ == "__main__":
> +    import logging
> +    logging.basicConfig(level=logging.DEBUG)
> +    unittest.main()


-- 
https://code.launchpad.net/~juliank/update-notifier/esm/+merge/365288
Your team Ubuntu Core Development Team is requested to review the proposed merge of lp:~juliank/update-notifier/esm into lp:update-notifier.



More information about the Ubuntu-reviews mailing list