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!",
+        )

Reply via email to