public inbox for gentoo-commits@lists.gentoo.org
 help / color / mirror / Atom feed
* [gentoo-commits] proj/portage:master commit in: lib/portage/tests/news/
@ 2023-02-18 10:13 Sam James
  0 siblings, 0 replies; 7+ messages in thread
From: Sam James @ 2023-02-18 10:13 UTC (permalink / raw
  To: gentoo-commits

commit:     5718444c3d8f21cce336c744c9afe3901c22c5d8
Author:     Sam James <sam <AT> gentoo <DOT> org>
AuthorDate: Thu Feb 16 08:19:42 2023 +0000
Commit:     Sam James <sam <AT> gentoo <DOT> org>
CommitDate: Sat Feb 18 10:13:35 2023 +0000
URL:        https://gitweb.gentoo.org/proj/portage.git/commit/?id=5718444c

tests: news: add test cases for previous news regressions

These tests would've caught the 3 previous regressions we had in the news feature
over the last year.

Verified by reverting each of the relevant fix commits (see below) and confirming
the relevant tests start to fail then pass again once the fix is cherry-picked in isolation.

See: 0e56f99b34939bf38dcfc0f9edf43a51f6ccf3fe
See: f1d98b6dc36ff2b47c36427c9938999320352eb4
See: 1ffaa70544f34e93df24c0a175105a900bf272bf
Bug: https://bugs.gentoo.org/857669
Bug: https://bugs.gentoo.org/889330
Signed-off-by: Sam James <sam <AT> gentoo.org>

 lib/portage/tests/news/test_NewsItem.py | 329 +++++++++++++++++++++++++++++---
 1 file changed, 302 insertions(+), 27 deletions(-)

diff --git a/lib/portage/tests/news/test_NewsItem.py b/lib/portage/tests/news/test_NewsItem.py
index fc26bed79..b48074648 100644
--- a/lib/portage/tests/news/test_NewsItem.py
+++ b/lib/portage/tests/news/test_NewsItem.py
@@ -1,24 +1,22 @@
 # test_NewsItem.py -- Portage Unit Testing Functionality
-# Copyright 2007-2019 Gentoo Authors
+# Copyright 2007-2023 Gentoo Authors
 # Distributed under the terms of the GNU General Public License v2
 
-from portage import os
 from portage.tests import TestCase
-from portage.news import NewsItem
+from portage.news import NewsItem, NewsManager
 from portage.dbapi.virtual import fakedbapi
-from tempfile import mkstemp
 
 from dataclasses import dataclass
 from string import Template
-from typing import Optional
-from unittest.mock import mock_open, patch
+from typing import Optional, List
+from unittest.mock import MagicMock, mock_open, patch
 
 import textwrap
 
 # The specification for news items is GLEP 42 ("Critical News Reporting"):
 # https://www.gentoo.org/glep/glep-0042.html
 
-# TODO: port the real newsitem class to this?
+
 @dataclass
 class FakeNewsItem(NewsItem):
     title: str
@@ -28,9 +26,9 @@ class FakeNewsItem(NewsItem):
     revision: int
     news_item_format: str
     content: str
-    display_if_installed: Optional[list[str]] = None
-    display_if_profile: Optional[list[str]] = None
-    display_if_keyword: Optional[list[str]] = None
+    display_if_installed: Optional[List[str]] = None
+    display_if_profile: Optional[List[str]] = None
+    display_if_keyword: Optional[List[str]] = None
 
     item_template_header = Template(
         textwrap.dedent(
@@ -49,9 +47,11 @@ class FakeNewsItem(NewsItem):
         super().__init__(path="mocked_news", name=self.title)
 
     def isValid(self):
-        with patch('builtins.open', mock_open(read_data=str(self))):
+        with patch("builtins.open", mock_open(read_data=str(self))):
             return super().isValid()
 
+    # TODO: Migrate __str__ to NewsItem? NewsItem doesn't actually parse
+    # all fields right now though.
     def __str__(self) -> str:
         item = self.item_template_header.substitute(
             title=self.title,
@@ -71,8 +71,7 @@ class FakeNewsItem(NewsItem):
         for keyword in self.display_if_keyword:
             item += f"Display-If-Keyword: {keyword}\n"
 
-        item += "\n"
-        item += f"{self.content}"
+        item += f"\n{self.content}"
 
         return item
 
@@ -98,11 +97,11 @@ class NewsItemTestCase(TestCase):
 
     Please see the Gentoo YourSQL Upgrade Guide for instructions:
 
-        http://www.gentoo.org/doc/en/yoursql-upgrading.xml
+        https://gentoo.org/doc/en/yoursql-upgrading.xml
 
     Also see the official YourSQL documentation:
 
-        http://dev.yoursql.com/doc/refman/4.1/en/upgrading-from-4-0.html
+        https://dev.example.com/doc/refman/4.1/en/upgrading-from-4-0.html
 
     After upgrading, you should also recompile any packages which link
     against YourSQL:
@@ -115,7 +114,8 @@ class NewsItemTestCase(TestCase):
     }
 
     def setUp(self) -> None:
-        self.profile = "/var/db/repos/gentoo/profiles/default-linux/x86/2007.0/"
+        self.profile_base = "/var/db/repos/gentoo/profiles/default-linux"
+        self.profile = f"{self.profile_base}/x86/2007.0/"
         self.keywords = "x86"
         # Consumers only use ARCH, so avoid portage.settings by using a dict
         self.settings = {"ARCH": "x86"}
@@ -131,47 +131,208 @@ class NewsItemTestCase(TestCase):
 
         return FakeNewsItem(**news_args)
 
+    def testNewsManager(self):
+        vardb = MagicMock()
+        portdb = MagicMock()
+        portdb.repositories.mainRepoLocation = MagicMock(return_value="/tmp/repo")
+        portdb.settings.profile_path = "/tmp/repo/profiles/arch/amd64"
+
+        news_manager = NewsManager(portdb, vardb, portdb.portdir, portdb.portdir)
+        self.assertEqual(news_manager._profile_path, "arch/amd64")
+        self.assertNotEqual(news_manager._profile_path, "tmp/repo/profiles/arch/amd64")
+
     def testBasicNewsItem(self):
         # Simple test with no filter fields (Display-If-*)
         item = self._createNewsItem()
         self.assertTrue(item.isValid())
-        # relevant: self.assertTrue(...)
+        self.assertTrue(item.isRelevant(self.vardb, self.settings, self.profile))
+
+        # Does an invalid item fail? ("a" is not a valid package name)
+        item = self._createNewsItem({"display_if_installed": "a"})
+        self.assertFalse(item.isValid())
 
     def testDisplayIfProfile(self):
-        # First, just check the simple case of one profile matching ours.
-        item = self._createNewsItem({"display_if_profile": [self.profile]})
+        # We repeat all of these with the full profile path (including repo)
+        # and a relative path, as we've had issues there before.
+        for profile_prefix in ("", self.profile_base):
+            # First, just check the simple case of one profile matching ours.
+            item = self._createNewsItem(
+                {"display_if_profile": [profile_prefix + self.profile]}
+            )
+            self.assertTrue(item.isValid())
+            self.assertTrue(
+                item.isRelevant(
+                    self.vardb, self.settings, profile_prefix + self.profile
+                ),
+                msg=f"Expected {item} to be relevant, but it was not!",
+            )
+
+            # Test the negative case: what if the only profile listed
+            # does *not* match ours?
+            item = self._createNewsItem(
+                {"display_if_profile": [profile_prefix + "profiles/i-do-not-exist"]}
+            )
+            self.assertTrue(item.isValid())
+            self.assertFalse(
+                item.isRelevant(
+                    self.vardb, self.settings, profile_prefix + self.profile
+                ),
+                msg=f"Expected {item} to be irrelevant, but it was relevant!",
+            )
+
+            # What if several profiles are listed and we match one of them?
+            item = self._createNewsItem(
+                {
+                    "display_if_profile": [
+                        profile_prefix + self.profile,
+                        profile_prefix + f"{self.profile_base}/amd64/2023.0",
+                    ]
+                }
+            )
+            self.assertTrue(item.isValid())
+            self.assertTrue(
+                item.isRelevant(
+                    self.vardb, self.settings, profile_prefix + self.profile
+                ),
+                msg=f"Expected {item} to be relevant, but it was not!",
+            )
+
+            # What if several profiles are listed and we match none of them?
+            item = self._createNewsItem(
+                {
+                    "display_if_profile": [
+                        profile_prefix + f"{self.profile_base}/x86/2023.0",
+                        profile_prefix + f"{self.profile_base}/amd64/2023.0",
+                    ]
+                }
+            )
+            self.assertTrue(item.isValid())
+            self.assertFalse(
+                item.isRelevant(
+                    self.vardb, self.settings, profile_prefix + self.profile
+                ),
+                msg=f"Expected {item} to be irrelevant, but it was relevant!",
+            )
+
+    def testDisplayIfInstalled(self):
+        self.vardb.cpv_inject("sys-apps/portage-2.0", {"SLOT": "0"})
+
+        item = self._createNewsItem({"display_if_installed": ["sys-apps/portage"]})
         self.assertTrue(item.isValid())
         self.assertTrue(
             item.isRelevant(self.vardb, self.settings, self.profile),
             msg=f"Expected {item} to be relevant, but it was not!",
         )
 
-        # Test the negative case: what if the only profile listed does *not* match ours?
-        item = self._createNewsItem({"display_if_profile": ["profiles/i-do-not-exist"]})
+        # Test the negative case: a single Display-If-Installed listing
+        # a package we don't have.
+        item = self._createNewsItem(
+            {"display_if_installed": ["sys-apps/i-do-not-exist"]}
+        )
         self.assertTrue(item.isValid())
         self.assertFalse(
             item.isRelevant(self.vardb, self.settings, self.profile),
             msg=f"Expected {item} to be irrelevant, but it was relevant!",
         )
 
-    def testDisplayIfInstalled(self):
-        self.vardb.cpv_inject('sys-apps/portage-2.0', { 'SLOT' : "0" })
-        item = self._createNewsItem({"display_if_installed": ["sys-apps/portage"]})
+        # What about several packages and we have none of them installed?
+        item = self._createNewsItem(
+            {
+                "display_if_installed": [
+                    "dev-util/pkgcheck",
+                    "dev-util/pkgdev",
+                    "sys-apps/pkgcore",
+                ]
+            }
+        )
+        self.assertTrue(item.isValid())
+        self.assertFalse(
+            item.isRelevant(self.vardb, self.settings, self.profile),
+            msg=f"Expected {item} to be irrelevant, but it was relevant!",
+        )
+
+        # What about several packages and we have one of them installed?
+        self.vardb.cpv_inject("net-misc/openssh-9.2_p1", {"SLOT": "0"})
+        item = self._createNewsItem(
+            {
+                "display_if_installed": [
+                    "net-misc/openssh",
+                    "net-misc/dropbear",
+                ]
+            }
+        )
         self.assertTrue(item.isValid())
         self.assertTrue(
             item.isRelevant(self.vardb, self.settings, self.profile),
             msg=f"Expected {item} to be relevant, but it was not!",
         )
 
-        # Test the negative case: a single Display-If-Installed listing
-        # a package we don't have.
-        item = self._createNewsItem({"display_if_installed": ["sys-apps/i-do-not-exist"]})
+        # What about several packages and we have all of them installed?
+        # Note: we already have openssh added from the above test
+        self.vardb.cpv_inject("net-misc/dropbear-2022.83", {"SLOT": "0"})
+        item = self._createNewsItem(
+            {
+                "display_if_installed": [
+                    "net-misc/openssh",
+                    "net-misc/dropbear",
+                ]
+            }
+        )
+        self.assertTrue(item.isValid())
+        self.assertTrue(
+            item.isRelevant(self.vardb, self.settings, self.profile),
+            msg=f"Expected {item} to be relevant, but it was not!",
+        )
+
+        # What if we have a newer version of the listed package which
+        # shouldn't match the constraint?
+        self.vardb.cpv_inject("net-misc/openssh-9.2_p1", {"SLOT": "0"})
+        item = self._createNewsItem(
+            {
+                "display_if_installed": [
+                    "<net-misc/openssh-9.2_p1",
+                ]
+            }
+        )
         self.assertTrue(item.isValid())
         self.assertFalse(
             item.isRelevant(self.vardb, self.settings, self.profile),
             msg=f"Expected {item} to be irrelevant, but it was relevant!",
         )
 
+        # What if we have a newer version of the listed package which
+        # should match the constraint?
+        item = self._createNewsItem(
+            {
+                "display_if_installed": [
+                    ">=net-misc/openssh-9.2_p1",
+                ]
+            }
+        )
+        self.assertTrue(item.isValid())
+        self.assertTrue(
+            item.isRelevant(self.vardb, self.settings, self.profile),
+            msg=f"Expected {item} to be relevant, but it was not!",
+        )
+
+        # What if the item lists multiple packages and we have one of
+        # them installed, but not all?
+        # (Note that openssh is already "installed" by this point because
+        # of a previous test.)
+        item = self._createNewsItem(
+            {
+                "display_if_installed": [
+                    ">=net-misc/openssh-9.2_p1",
+                    "<net-misc/openssh-9.2_p1",
+                ]
+            }
+        )
+        self.assertTrue(item.isValid())
+        self.assertTrue(
+            item.isRelevant(self.vardb, self.settings, self.profile),
+            msg=f"Expected {item} to be relevant, but it was not!",
+        )
+
     def testDisplayIfKeyword(self):
         item = self._createNewsItem({"display_if_keyword": [self.keywords]})
         self.assertTrue(item.isValid())
@@ -187,3 +348,117 @@ class NewsItemTestCase(TestCase):
             item.isRelevant(self.vardb, self.settings, self.profile),
             msg=f"Expected {item} to be irrelevant, but it was relevant!",
         )
+
+        # What if several keywords are listed and we match one of them?
+        item = self._createNewsItem(
+            {"display_if_keyword": [self.keywords, "amd64", "~hppa"]}
+        )
+        self.assertTrue(item.isValid())
+        self.assertTrue(
+            item.isRelevant(self.vardb, self.settings, self.profile),
+            msg=f"Expected {item} to be relevant, but it was not!",
+        )
+
+        # What if several keywords are listed and we match none of them?
+        item = self._createNewsItem({"display_if_keyword": ["amd64", "~hppa"]})
+        self.assertTrue(item.isValid())
+        self.assertFalse(
+            item.isRelevant(self.vardb, self.settings, self.profile),
+            msg=f"Expected {item} to be irrelevant, but it was relevant!",
+        )
+
+        # What if the ~keyword (testing) keyword is listed but we're keyword (stable)?
+        item = self._createNewsItem(
+            {
+                "display_if_keyword": [
+                    f"~{self.keywords}",
+                ]
+            }
+        )
+        self.assertTrue(item.isValid())
+        self.assertFalse(
+            item.isRelevant(self.vardb, self.settings, self.profile),
+            msg=f"Expected {item} to be irrelevant, but it was relevant!",
+        )
+
+        # What if the stable keyword is listed but we're ~keyword (testing)?
+        item = self._createNewsItem(
+            {
+                "display_if_keyword": [
+                    f"{self.keywords}",
+                ]
+            }
+        )
+        self.assertTrue(item.isValid())
+        self.assertTrue(
+            item.isRelevant(self.vardb, self.settings, self.profile),
+            msg=f"Expected {item} to be relevant, but it was not!",
+        )
+
+    def testMultipleRestrictions(self):
+        # GLEP 42 specifies an algorithm for how combining restrictions
+        # should work. See https://www.gentoo.org/glep/glep-0042.html#news-item-headers.
+        # Different types of Display-If-* are ANDed, not ORed.
+
+        # What if there's a Display-If-Keyword that matches and a
+        # Display-If-Installed which does too?
+        self.vardb.cpv_inject("sys-apps/portage-2.0", {"SLOT": "0"})
+        item = self._createNewsItem(
+            {
+                "display_if_keyword": [self.keywords],
+                "display_if_installed": ["sys-apps/portage"]
+             }
+        )
+        self.assertTrue(item.isValid())
+        self.assertTrue(
+            item.isRelevant(self.vardb, self.settings, self.profile),
+            msg=f"Expected {item} to be relevant, but it was not!",
+        )
+
+        # What if there's a Display-If-Keyword that matches and a
+        # Display-If-Installed which doesn't?
+        item = self._createNewsItem(
+            {
+                "display_if_keyword": [self.keywords],
+                "display_if_installed": ["sys-apps/i-do-not-exist"]
+             }
+        )
+        self.assertTrue(item.isValid())
+        self.assertFalse(
+            item.isRelevant(self.vardb, self.settings, self.profile),
+            msg=f"Expected {item} to be irrelevant, but it was relevant!",
+        )
+
+        # What if there's a Display-If-{Installed,Keyword,Profile} and
+        # they all match?
+        # (Note that sys-apps/portage is already "installed" by this point
+        # because of the above test.)
+        item = self._createNewsItem(
+            {
+                "display_if_keyword": [self.keywords],
+                "display_if_installed": ["sys-apps/portage"],
+                "display_if_profile": [self.profile],
+             }
+        )
+        self.assertTrue(item.isValid())
+        self.assertTrue(
+            item.isRelevant(self.vardb, self.settings, self.profile),
+            msg=f"Expected {item} to be relevant, but it was not!",
+        )
+
+        # What if there's a Display-If-{Installed,Keyword,Profile} and
+        # none of them match?
+        # (Note that sys-apps/portage is already "installed" by this point
+        # because of the above test.)
+        item = self._createNewsItem(
+            {
+                "display_if_keyword": ["i-do-not-exist"],
+                "display_if_installed": ["sys-apps/i-do-not-exist"],
+                "display_if_profile": [self.profile_base + "/i-do-not-exist"],
+             }
+        )
+        self.assertTrue(item.isValid())
+        self.assertFalse(
+            item.isRelevant(self.vardb, self.settings, self.profile),
+            msg=f"Expected {item} to be irrelevant, but it was relevant!",
+        )


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [gentoo-commits] proj/portage:master commit in: lib/portage/tests/news/
@ 2023-02-18 10:13 Sam James
  0 siblings, 0 replies; 7+ messages in thread
From: Sam James @ 2023-02-18 10:13 UTC (permalink / raw
  To: gentoo-commits

commit:     ebf78db6d9bb72f5ca6538ad5cc94ec297f9aa2f
Author:     Sam James <sam <AT> gentoo <DOT> org>
AuthorDate: Thu Feb 16 07:42:31 2023 +0000
Commit:     Sam James <sam <AT> gentoo <DOT> org>
CommitDate: Sat Feb 18 10:13:35 2023 +0000
URL:        https://gitweb.gentoo.org/proj/portage.git/commit/?id=ebf78db6

tests: news: add trivial no-filter test

Add a basic test case for when no filter fields are used
(no Display-If-*).

Bug: https://bugs.gentoo.org/889330
Signed-off-by: Sam James <sam <AT> gentoo.org>

 lib/portage/tests/news/test_NewsItem.py | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/lib/portage/tests/news/test_NewsItem.py b/lib/portage/tests/news/test_NewsItem.py
index e6f9a08b3..ba9fa0035 100644
--- a/lib/portage/tests/news/test_NewsItem.py
+++ b/lib/portage/tests/news/test_NewsItem.py
@@ -137,6 +137,13 @@ class NewsItemTestCase(TestCase):
 
         return FakeNewsItem(**news_args)
 
+    def testBasicNewsItem(self):
+        # Simple test with no filter fields (Display-If-*)
+        try:
+            item = self._processItem(str(self._createNewsItem()))
+        finally:
+            os.unlink(item.path)
+
     def testDisplayIfProfile(self):
         tmpItem = self._createNewsItem({"display_if_profile": [self.profile]})
 


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [gentoo-commits] proj/portage:master commit in: lib/portage/tests/news/
@ 2023-02-18 10:13 Sam James
  0 siblings, 0 replies; 7+ messages in thread
From: Sam James @ 2023-02-18 10:13 UTC (permalink / raw
  To: gentoo-commits

commit:     b369e8296fcc802178f5e178ccc0e43307c789bd
Author:     Sam James <sam <AT> gentoo <DOT> org>
AuthorDate: Thu Feb 16 07:41:19 2023 +0000
Commit:     Sam James <sam <AT> gentoo <DOT> org>
CommitDate: Sat Feb 18 10:13:34 2023 +0000
URL:        https://gitweb.gentoo.org/proj/portage.git/commit/?id=b369e829

tests: news: mention GLEP 42 explicitly

Bug: https://bugs.gentoo.org/889330
Signed-off-by: Sam James <sam <AT> gentoo.org>

 lib/portage/tests/news/test_NewsItem.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/portage/tests/news/test_NewsItem.py b/lib/portage/tests/news/test_NewsItem.py
index ba912a525..e6f9a08b3 100644
--- a/lib/portage/tests/news/test_NewsItem.py
+++ b/lib/portage/tests/news/test_NewsItem.py
@@ -14,6 +14,9 @@ from typing import Optional
 
 import textwrap
 
+# The specification for news items is GLEP 42 ("Critical News Reporting"):
+# https://www.gentoo.org/glep/glep-0042.html
+
 # TODO(antarus) Make newsitem use a loader so we can load using a string instead of a tempfile
 
 


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [gentoo-commits] proj/portage:master commit in: lib/portage/tests/news/
@ 2023-02-18 10:13 Sam James
  0 siblings, 0 replies; 7+ messages in thread
From: Sam James @ 2023-02-18 10:13 UTC (permalink / raw
  To: gentoo-commits

commit:     b7bd20a83676fa29ac92a9790cee84c68148beb0
Author:     Sam James <sam <AT> gentoo <DOT> org>
AuthorDate: Thu Feb 16 08:02:14 2023 +0000
Commit:     Sam James <sam <AT> gentoo <DOT> org>
CommitDate: Sat Feb 18 10:13:35 2023 +0000
URL:        https://gitweb.gentoo.org/proj/portage.git/commit/?id=b7bd20a8

tests: news: add failing Display-If-Installed test

This shows that our current Display-If-Installed test
doesn't work properly, as it'll pass with any package
value.

Bug: https://bugs.gentoo.org/889330
Signed-off-by: Sam James <sam <AT> gentoo.org>

 lib/portage/tests/news/test_NewsItem.py | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/lib/portage/tests/news/test_NewsItem.py b/lib/portage/tests/news/test_NewsItem.py
index ba9fa0035..9324a7d18 100644
--- a/lib/portage/tests/news/test_NewsItem.py
+++ b/lib/portage/tests/news/test_NewsItem.py
@@ -170,6 +170,18 @@ class NewsItemTestCase(TestCase):
         finally:
             os.unlink(item.path)
 
+        tmpItem = self._createNewsItem({"display_if_installed": ["sys-apps/i-do-not-exist"]})
+
+        try:
+            item = self._processItem(str(tmpItem))
+            self.assertTrue(item.isValid())
+            self.assertFalse(
+                item.isRelevant(self.vardb, self.settings, self.profile),
+                msg=f"Expected {tmpItem} to be irrelevant, but it was relevant!",
+            )
+        finally:
+            os.unlink(item.path)
+
     def testDisplayIfKeyword(self):
         tmpItem = self._createNewsItem({"display_if_keyword": [self.keywords]})
 


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [gentoo-commits] proj/portage:master commit in: lib/portage/tests/news/
@ 2023-02-18 10:13 Sam James
  0 siblings, 0 replies; 7+ messages in thread
From: Sam James @ 2023-02-18 10:13 UTC (permalink / raw
  To: gentoo-commits

commit:     c074e090e057a6d4303b492493c8cd5e9a180107
Author:     Sam James <sam <AT> gentoo <DOT> org>
AuthorDate: Thu Feb 16 07:30:08 2023 +0000
Commit:     Sam James <sam <AT> gentoo <DOT> org>
CommitDate: Sat Feb 18 10:13:34 2023 +0000
URL:        https://gitweb.gentoo.org/proj/portage.git/commit/?id=c074e090

tests: news: use templates for mock news items

This is some prep work to modernise the tests a bit
by using dataclasses and idiomatic Python string/template
substitution.

The key point here is the changes facilitate testing
with various combinations of fields, including multiple
e.g. Display-If-Installed, which we couldn't do cleanly
before.

Bug: https://bugs.gentoo.org/889330
Signed-off-by: Sam James <sam <AT> gentoo.org>

 lib/portage/tests/news/test_NewsItem.py | 149 ++++++++++++++++++++++++--------
 1 file changed, 111 insertions(+), 38 deletions(-)

diff --git a/lib/portage/tests/news/test_NewsItem.py b/lib/portage/tests/news/test_NewsItem.py
index d5fbc10e0..fcfc06b13 100644
--- a/lib/portage/tests/news/test_NewsItem.py
+++ b/lib/portage/tests/news/test_NewsItem.py
@@ -8,45 +8,115 @@ from portage.news import NewsItem
 from portage.dbapi.virtual import testdbapi
 from tempfile import mkstemp
 
+from dataclasses import dataclass
+from string import Template
+from typing import Optional
+
+import textwrap
+
 # TODO(antarus) Make newsitem use a loader so we can load using a string instead of a tempfile
 
 
+# TODO: port the real newsitem class to this?
+@dataclass
+class FakeNewsItem:
+    title: str
+    author: str
+    content_type: str
+    posted: str
+    revision: int
+    news_item_format: str
+    content: str
+    display_if_installed: Optional[list[str]] = None
+    display_if_profile: Optional[list[str]] = None
+    display_if_keyword: Optional[list[str]] = None
+
+    item_template_header = Template(
+        textwrap.dedent(
+            """
+        Title: ${title}
+        Author: ${author}
+        Content-Type: ${content_type}
+        Posted: ${posted}
+        Revision: ${revision}
+        News-Item-Format: ${news_item_format}
+        """
+        )
+    )
+
+    def __post_init__(self):
+        if not any(
+            [self.display_if_installed, self.display_if_profile, self.display_if_keyword]
+        ):
+            raise ValueError(
+                "At least one-of Display-If-Installed, Display-If-Profile, or Display-If-Arch must be set!"
+            )
+
+    def __str__(self) -> str:
+        item = self.item_template_header.substitute(
+            title=self.title,
+            author=self.author,
+            content_type=self.content_type,
+            posted=self.posted,
+            revision=self.revision,
+            news_item_format=self.news_item_format,
+        )
+
+        for package in self.display_if_installed:
+            item += f"Display-If-Installed: {package}\n"
+
+        for profile in self.display_if_profile:
+            item += f"Display-If-Profile: {profile}\n"
+
+        for keyword in self.display_if_keyword:
+            item += f"Display-If-Keyword: {keyword}\n"
+
+        item += "\n"
+        item += f"{self.content}"
+
+        return item
+
+
 class NewsItemTestCase(TestCase):
     """These tests suck: they use your running config instead of making their own"""
 
-    fakeItem = """
-Title: YourSQL Upgrades from 4.0 to 4.1
-Author: Ciaran McCreesh <ciaranm@gentoo.org>
-Content-Type: text/plain
-Posted: 01-Nov-2005
-Revision: 1
-News-Item-Format: 1.0
-#Display-If-Installed:
-#Display-If-Profile:
-#Display-If-Arch:
+    # Default values for testing
+    placeholders = {
+        "title": "YourSQL Upgrades from 4.0 to 4.1",
+        "author": "Ciaran McCreesh <ciaranm@gentoo.org>",
+        "content_type": "Content-Type: text/plain",
+        "posted": "01-Nov-2005",
+        "revision": 1,
+        "news_item_format": "1.0",
+        "display_if_installed": [],
+        "display_if_profile": [],
+        "display_if_keyword": [],
+        "content": textwrap.dedent(
+            """
+    YourSQL databases created using YourSQL version 4.0 are incompatible
+    with YourSQL version 4.1 or later. There is no reliable way to
+    automate the database format conversion, so action from the system
+    administrator is required before an upgrade can take place.
 
-YourSQL databases created using YourSQL version 4.0 are incompatible
-with YourSQL version 4.1 or later. There is no reliable way to
-automate the database format conversion, so action from the system
-administrator is required before an upgrade can take place.
+    Please see the Gentoo YourSQL Upgrade Guide for instructions:
 
-Please see the Gentoo YourSQL Upgrade Guide for instructions:
+        http://www.gentoo.org/doc/en/yoursql-upgrading.xml
 
-    http://www.gentoo.org/doc/en/yoursql-upgrading.xml
+    Also see the official YourSQL documentation:
 
-Also see the official YourSQL documentation:
+        http://dev.yoursql.com/doc/refman/4.1/en/upgrading-from-4-0.html
 
-    http://dev.yoursql.com/doc/refman/4.1/en/upgrading-from-4-0.html
+    After upgrading, you should also recompile any packages which link
+    against YourSQL:
 
-After upgrading, you should also recompile any packages which link
-against YourSQL:
+        revdep-rebuild --library=libyoursqlclient.so.12
 
-    revdep-rebuild --library=libyoursqlclient.so.12
+    The revdep-rebuild tool is provided by app-portage/gentoolkit.
+    """
+        ),
+    }
 
-The revdep-rebuild tool is provided by app-portage/gentoolkit.
-"""
-
-    def setUp(self):
+    def setUp(self) -> None:
         self.profile = "/var/db/repos/gentoo/profiles/default-linux/x86/2007.0/"
         self.keywords = "x86"
         # Use fake/test dbapi to avoid slow tests
@@ -55,12 +125,19 @@ The revdep-rebuild tool is provided by app-portage/gentoolkit.
         # Consumers only use ARCH, so avoid portage.settings by using a dict
         self.settings = {"ARCH": "x86"}
 
+    def _createNewsItem(self, *kwargs) -> FakeNewsItem:
+        # Use our placeholders unless overridden
+        news_args = self.placeholders.copy()
+        # Substitute in what we're given to allow for easily passing
+        # just custom values.
+        news_args.update(*kwargs)
+
+        return FakeNewsItem(**news_args)
+
     def testDisplayIfProfile(self):
-        tmpItem = self.fakeItem[:].replace(
-            "#Display-If-Profile:", f"Display-If-Profile: {self.profile}"
-        )
+        tmpItem = self._createNewsItem({"display_if_profile": [self.profile]})
 
-        item = self._processItem(tmpItem)
+        item = self._processItem(str(tmpItem))
         try:
             self.assertTrue(
                 item.isRelevant(self.vardb, self.settings, self.profile),
@@ -70,12 +147,10 @@ The revdep-rebuild tool is provided by app-portage/gentoolkit.
             os.unlink(item.path)
 
     def testDisplayIfInstalled(self):
-        tmpItem = self.fakeItem[:].replace(
-            "#Display-If-Installed:", f"Display-If-Installed: {'sys-apps/portage'}"
-        )
+        tmpItem = self._createNewsItem({"display_if_installed": ["sys-apps/portage"]})
 
         try:
-            item = self._processItem(tmpItem)
+            item = self._processItem(str(tmpItem))
             self.assertTrue(
                 item.isRelevant(self.vardb, self.settings, self.profile),
                 msg=f"Expected {tmpItem} to be relevant, but it was not!",
@@ -84,12 +159,10 @@ The revdep-rebuild tool is provided by app-portage/gentoolkit.
             os.unlink(item.path)
 
     def testDisplayIfKeyword(self):
-        tmpItem = self.fakeItem[:].replace(
-            "#Display-If-Keyword:", f"Display-If-Keyword: {self.keywords}"
-        )
+        tmpItem = self._createNewsItem({"display_if_keyword": [self.keywords]})
 
         try:
-            item = self._processItem(tmpItem)
+            item = self._processItem(str(tmpItem))
             self.assertTrue(
                 item.isRelevant(self.vardb, self.settings, self.profile),
                 msg=f"Expected {tmpItem} to be relevant, but it was not!",
@@ -97,7 +170,7 @@ The revdep-rebuild tool is provided by app-portage/gentoolkit.
         finally:
             os.unlink(item.path)
 
-    def _processItem(self, item):
+    def _processItem(self, item) -> NewsItem:
         filename = None
         fd, filename = mkstemp()
         f = os.fdopen(fd, "w")


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [gentoo-commits] proj/portage:master commit in: lib/portage/tests/news/
@ 2023-02-18 10:13 Sam James
  0 siblings, 0 replies; 7+ messages in thread
From: Sam James @ 2023-02-18 10:13 UTC (permalink / raw
  To: gentoo-commits

commit:     1ca9ff926ae0cc7778041d21999c59340bd2d5cb
Author:     Sam James <sam <AT> gentoo <DOT> org>
AuthorDate: Thu Feb 16 08:35:58 2023 +0000
Commit:     Sam James <sam <AT> gentoo <DOT> org>
CommitDate: Sat Feb 18 10:13:35 2023 +0000
URL:        https://gitweb.gentoo.org/proj/portage.git/commit/?id=1ca9ff92

tests: news: use mocked NewsItem

This eliminates a lot of boilerplate from each test.

Bug: https://bugs.gentoo.org/889330
Signed-off-by: Sam James <sam <AT> gentoo.org>

 lib/portage/tests/news/test_NewsItem.py | 124 ++++++++++++++------------------
 1 file changed, 54 insertions(+), 70 deletions(-)

diff --git a/lib/portage/tests/news/test_NewsItem.py b/lib/portage/tests/news/test_NewsItem.py
index 53a9093de..fc26bed79 100644
--- a/lib/portage/tests/news/test_NewsItem.py
+++ b/lib/portage/tests/news/test_NewsItem.py
@@ -11,18 +11,16 @@ from tempfile import mkstemp
 from dataclasses import dataclass
 from string import Template
 from typing import Optional
+from unittest.mock import mock_open, patch
 
 import textwrap
 
 # The specification for news items is GLEP 42 ("Critical News Reporting"):
 # https://www.gentoo.org/glep/glep-0042.html
 
-# TODO(antarus) Make newsitem use a loader so we can load using a string instead of a tempfile
-
-
 # TODO: port the real newsitem class to this?
 @dataclass
-class FakeNewsItem:
+class FakeNewsItem(NewsItem):
     title: str
     author: str
     content_type: str
@@ -48,12 +46,11 @@ class FakeNewsItem:
     )
 
     def __post_init__(self):
-        if not any(
-            [self.display_if_installed, self.display_if_profile, self.display_if_keyword]
-        ):
-            raise ValueError(
-                "At least one-of Display-If-Installed, Display-If-Profile, or Display-If-Arch must be set!"
-            )
+        super().__init__(path="mocked_news", name=self.title)
+
+    def isValid(self):
+        with patch('builtins.open', mock_open(read_data=str(self))):
+            return super().isValid()
 
     def __str__(self) -> str:
         item = self.item_template_header.substitute(
@@ -136,70 +133,57 @@ class NewsItemTestCase(TestCase):
 
     def testBasicNewsItem(self):
         # Simple test with no filter fields (Display-If-*)
-        try:
-            item = self._processItem(str(self._createNewsItem()))
-        finally:
-            os.unlink(item.path)
+        item = self._createNewsItem()
+        self.assertTrue(item.isValid())
+        # relevant: self.assertTrue(...)
 
     def testDisplayIfProfile(self):
-        tmpItem = self._createNewsItem({"display_if_profile": [self.profile]})
-
-        item = self._processItem(str(tmpItem))
-        try:
-            self.assertTrue(item.isValid())
-            self.assertTrue(
-                item.isRelevant(self.vardb, self.settings, self.profile),
-                msg=f"Expected {tmpItem} to be relevant, but it was not!",
-            )
-        finally:
-            os.unlink(item.path)
+        # First, just check the simple case of one profile matching ours.
+        item = self._createNewsItem({"display_if_profile": [self.profile]})
+        self.assertTrue(item.isValid())
+        self.assertTrue(
+            item.isRelevant(self.vardb, self.settings, self.profile),
+            msg=f"Expected {item} to be relevant, but it was not!",
+        )
+
+        # Test the negative case: what if the only profile listed does *not* match ours?
+        item = self._createNewsItem({"display_if_profile": ["profiles/i-do-not-exist"]})
+        self.assertTrue(item.isValid())
+        self.assertFalse(
+            item.isRelevant(self.vardb, self.settings, self.profile),
+            msg=f"Expected {item} to be irrelevant, but it was relevant!",
+        )
 
     def testDisplayIfInstalled(self):
         self.vardb.cpv_inject('sys-apps/portage-2.0', { 'SLOT' : "0" })
-        tmpItem = self._createNewsItem({"display_if_installed": ["sys-apps/portage"]})
-
-        try:
-            item = self._processItem(str(tmpItem))
-            self.assertTrue(item.isValid())
-            self.assertTrue(
-                item.isRelevant(self.vardb, self.settings, self.profile),
-                msg=f"Expected {tmpItem} to be relevant, but it was not!",
-            )
-        finally:
-            os.unlink(item.path)
-
-        tmpItem = self._createNewsItem({"display_if_installed": ["sys-apps/i-do-not-exist"]})
-
-        try:
-            item = self._processItem(str(tmpItem))
-            self.assertTrue(item.isValid())
-            self.assertFalse(
-                item.isRelevant(self.vardb, self.settings, self.profile),
-                msg=f"Expected {tmpItem} to be irrelevant, but it was relevant!",
-            )
-        finally:
-            os.unlink(item.path)
+        item = self._createNewsItem({"display_if_installed": ["sys-apps/portage"]})
+        self.assertTrue(item.isValid())
+        self.assertTrue(
+            item.isRelevant(self.vardb, self.settings, self.profile),
+            msg=f"Expected {item} to be relevant, but it was not!",
+        )
+
+        # Test the negative case: a single Display-If-Installed listing
+        # a package we don't have.
+        item = self._createNewsItem({"display_if_installed": ["sys-apps/i-do-not-exist"]})
+        self.assertTrue(item.isValid())
+        self.assertFalse(
+            item.isRelevant(self.vardb, self.settings, self.profile),
+            msg=f"Expected {item} to be irrelevant, but it was relevant!",
+        )
 
     def testDisplayIfKeyword(self):
-        tmpItem = self._createNewsItem({"display_if_keyword": [self.keywords]})
-
-        try:
-            item = self._processItem(str(tmpItem))
-            self.assertTrue(item.isValid())
-            self.assertTrue(
-                item.isRelevant(self.vardb, self.settings, self.profile),
-                msg=f"Expected {tmpItem} to be relevant, but it was not!",
-            )
-        finally:
-            os.unlink(item.path)
-
-    def _processItem(self, item) -> NewsItem:
-        filename = None
-        fd, filename = mkstemp()
-        f = os.fdopen(fd, "w")
-        f.write(item)
-        f.close()
-        try:
-            return NewsItem(filename, 0)
-        except TypeError:
-            self.fail(f"Error while processing news item {filename}")
+        item = self._createNewsItem({"display_if_keyword": [self.keywords]})
+        self.assertTrue(item.isValid())
+        self.assertTrue(
+            item.isRelevant(self.vardb, self.settings, self.profile),
+            msg=f"Expected {item} to be relevant, but it was not!",
+        )
+
+        # Test the negative case: a keyword we don't have set.
+        item = self._createNewsItem({"display_if_keyword": ["fake-keyword"]})
+        self.assertTrue(item.isValid())
+        self.assertFalse(
+            item.isRelevant(self.vardb, self.settings, self.profile),
+            msg=f"Expected {item} to be irrelevant, but it was relevant!",
+        )


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [gentoo-commits] proj/portage:master commit in: lib/portage/tests/news/
@ 2023-02-18 10:13 Sam James
  0 siblings, 0 replies; 7+ messages in thread
From: Sam James @ 2023-02-18 10:13 UTC (permalink / raw
  To: gentoo-commits

commit:     23b0bfae026a7bbb31e544eed9b8346ef85fc610
Author:     Sam James <sam <AT> gentoo <DOT> org>
AuthorDate: Thu Feb 16 07:35:05 2023 +0000
Commit:     Sam James <sam <AT> gentoo <DOT> org>
CommitDate: Sat Feb 18 10:13:34 2023 +0000
URL:        https://gitweb.gentoo.org/proj/portage.git/commit/?id=23b0bfae

tests: news: always check for validity of generated news item

Previously, we'd ignore invalid news items in these tests
because they're intended to check for the respective
'relevance' field. Fix that.

Bug: https://bugs.gentoo.org/889330
Signed-off-by: Sam James <sam <AT> gentoo.org>

 lib/portage/tests/news/test_NewsItem.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/portage/tests/news/test_NewsItem.py b/lib/portage/tests/news/test_NewsItem.py
index fcfc06b13..ba912a525 100644
--- a/lib/portage/tests/news/test_NewsItem.py
+++ b/lib/portage/tests/news/test_NewsItem.py
@@ -139,6 +139,7 @@ class NewsItemTestCase(TestCase):
 
         item = self._processItem(str(tmpItem))
         try:
+            self.assertTrue(item.isValid())
             self.assertTrue(
                 item.isRelevant(self.vardb, self.settings, self.profile),
                 msg=f"Expected {tmpItem} to be relevant, but it was not!",
@@ -151,6 +152,7 @@ class NewsItemTestCase(TestCase):
 
         try:
             item = self._processItem(str(tmpItem))
+            self.assertTrue(item.isValid())
             self.assertTrue(
                 item.isRelevant(self.vardb, self.settings, self.profile),
                 msg=f"Expected {tmpItem} to be relevant, but it was not!",
@@ -163,6 +165,7 @@ class NewsItemTestCase(TestCase):
 
         try:
             item = self._processItem(str(tmpItem))
+            self.assertTrue(item.isValid())
             self.assertTrue(
                 item.isRelevant(self.vardb, self.settings, self.profile),
                 msg=f"Expected {tmpItem} to be relevant, but it was not!",


^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2023-02-18 10:13 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-18 10:13 [gentoo-commits] proj/portage:master commit in: lib/portage/tests/news/ Sam James
  -- strict thread matches above, loose matches on Subject: below --
2023-02-18 10:13 Sam James
2023-02-18 10:13 Sam James
2023-02-18 10:13 Sam James
2023-02-18 10:13 Sam James
2023-02-18 10:13 Sam James
2023-02-18 10:13 Sam James

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox