From 3f6194379f9b4471b673733f7d7880cc1ca2d438 Mon Sep 17 00:00:00 2001 From: Sam James Date: Thu, 16 Feb 2023 08:19:42 +0000 Subject: [PATCH] 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 --- 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 fc26bed79c..b480746489 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 @@ def __post_init__(self): 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 @@ def __str__(self) -> str: 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 @@ def _createNewsItem(self, *kwargs) -> FakeNewsItem: 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.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", + "