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

commit:     bed3311d84455ca49b45dc3146ecaf74d6ee8dc1
Author:     Sam James <sam <AT> gentoo <DOT> org>
AuthorDate: Thu Feb 16 08:09:43 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=bed3311d

tests: news: drop usage of incomplete testdbapi, port to fakedbapi

It's been TODO for many years (see a4acda03bac43cc972dfaf9fda4d5210860d3d93)
and it covered up a problem with Display-If-Installed's test not actually
asserting if the package was installed or not.

Now e.g. dbapi.match() gives a proper result.

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

 lib/portage/dbapi/virtual.py            | 19 -------------------
 lib/portage/tests/news/test_NewsItem.py | 10 ++++------
 2 files changed, 4 insertions(+), 25 deletions(-)

diff --git a/lib/portage/dbapi/virtual.py b/lib/portage/dbapi/virtual.py
index bba45f47d..2410ba1ed 100644
--- a/lib/portage/dbapi/virtual.py
+++ b/lib/portage/dbapi/virtual.py
@@ -212,22 +212,3 @@ class fakedbapi(dbapi):
         if metadata is None:
             raise KeyError(cpv)
         metadata.update(values)
-
-
-class testdbapi:
-    """A dbapi instance with completely fake functions to get by hitting disk
-    TODO(antarus):
-    This class really needs to be rewritten to have better stubs; but these work for now.
-    The dbapi classes themselves need unit tests...and that will be a lot of work.
-    """
-
-    def __init__(self):
-        self.cpvs = {}
-
-        def f(*args, **kwargs):
-            return True
-
-        fake_api = dir(dbapi)
-        for call in fake_api:
-            if not hasattr(self, call):
-                setattr(self, call, f)

diff --git a/lib/portage/tests/news/test_NewsItem.py b/lib/portage/tests/news/test_NewsItem.py
index 9324a7d18..53a9093de 100644
--- a/lib/portage/tests/news/test_NewsItem.py
+++ b/lib/portage/tests/news/test_NewsItem.py
@@ -5,7 +5,7 @@
 from portage import os
 from portage.tests import TestCase
 from portage.news import NewsItem
-from portage.dbapi.virtual import testdbapi
+from portage.dbapi.virtual import fakedbapi
 from tempfile import mkstemp
 
 from dataclasses import dataclass
@@ -81,8 +81,6 @@ class FakeNewsItem:
 
 
 class NewsItemTestCase(TestCase):
-    """These tests suck: they use your running config instead of making their own"""
-
     # Default values for testing
     placeholders = {
         "title": "YourSQL Upgrades from 4.0 to 4.1",
@@ -122,11 +120,10 @@ class NewsItemTestCase(TestCase):
     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
-        self.vardb = testdbapi()
-        # self.vardb.inject_cpv('sys-apps/portage-2.0', { 'SLOT' : 0 })
         # Consumers only use ARCH, so avoid portage.settings by using a dict
         self.settings = {"ARCH": "x86"}
+        # Use fake/test dbapi to avoid slow tests
+        self.vardb = fakedbapi(self.settings)
 
     def _createNewsItem(self, *kwargs) -> FakeNewsItem:
         # Use our placeholders unless overridden
@@ -158,6 +155,7 @@ class NewsItemTestCase(TestCase):
             os.unlink(item.path)
 
     def testDisplayIfInstalled(self):
+        self.vardb.cpv_inject('sys-apps/portage-2.0', { 'SLOT' : "0" })
         tmpItem = self._createNewsItem({"display_if_installed": ["sys-apps/portage"]})
 
         try:


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

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

commit:     f6f7f8138b09c079bff53ea0869c0d5af26caf4c
Author:     Sam James <sam <AT> gentoo <DOT> org>
AuthorDate: Sat Feb 18 09:37:12 2023 +0000
Commit:     Sam James <sam <AT> gentoo <DOT> org>
CommitDate: Sat Feb 18 10:13:46 2023 +0000
URL:        https://gitweb.gentoo.org/proj/portage.git/commit/?id=f6f7f813

tests: news: reduce boilerplate further

Bug: https://bugs.gentoo.org/889330
Closes: https://github.com/gentoo/portage/pull/989
Signed-off-by: Sam James <sam <AT> gentoo.org>

 lib/portage/dbapi/virtual.py            |   2 +-
 lib/portage/tests/news/test_NewsItem.py | 173 ++++++++++----------------------
 2 files changed, 55 insertions(+), 120 deletions(-)

diff --git a/lib/portage/dbapi/virtual.py b/lib/portage/dbapi/virtual.py
index 2410ba1ed..8e1f14041 100644
--- a/lib/portage/dbapi/virtual.py
+++ b/lib/portage/dbapi/virtual.py
@@ -1,4 +1,4 @@
-# Copyright 1998-2020 Gentoo Authors
+# Copyright 1998-2023 Gentoo Authors
 # Distributed under the terms of the GNU General Public License v2
 
 from portage.dbapi import dbapi

diff --git a/lib/portage/tests/news/test_NewsItem.py b/lib/portage/tests/news/test_NewsItem.py
index b48074648..27fee1bb1 100644
--- a/lib/portage/tests/news/test_NewsItem.py
+++ b/lib/portage/tests/news/test_NewsItem.py
@@ -131,6 +131,25 @@ class NewsItemTestCase(TestCase):
 
         return FakeNewsItem(**news_args)
 
+    def _checkAndCreateNewsItem(
+        self, news_args: dict, relevant: bool = True, reason: str = ""
+    ) -> FakeNewsItem:
+        return self._checkNewsItem(self._createNewsItem(news_args), relevant, reason)
+
+    def _checkNewsItem(self, item: NewsItem, relevant: bool = True, reason: str = ""):
+        self.assertTrue(item.isValid())
+
+        if relevant:
+            self.assertTrue(
+                item.isRelevant(self.vardb, self.settings, self.profile),
+                msg=f"Expected {item} to be relevant, but it was not!",
+            )
+        else:
+            self.assertFalse(
+                item.isRelevant(self.vardb, self.settings, self.profile),
+                msg=f"Expected {item} to be irrelevant, but it was relevant!",
+            )
+
     def testNewsManager(self):
         vardb = MagicMock()
         portdb = MagicMock()
@@ -154,6 +173,8 @@ class NewsItemTestCase(TestCase):
     def testDisplayIfProfile(self):
         # We repeat all of these with the full profile path (including repo)
         # and a relative path, as we've had issues there before.
+        # Note that we can't use _checkNewsItem() here as we override the
+        # profile value passed to isRelevant.
         for profile_prefix in ("", self.profile_base):
             # First, just check the simple case of one profile matching ours.
             item = self._createNewsItem(
@@ -217,43 +238,29 @@ class NewsItemTestCase(TestCase):
     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!",
-        )
+        self._checkAndCreateNewsItem({"display_if_installed": ["sys-apps/portage"]})
 
         # 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!",
+        self._checkAndCreateNewsItem(
+            {"display_if_installed": ["sys-apps/i-do-not-exist"]}, False
         )
 
         # What about several packages and we have none of them installed?
-        item = self._createNewsItem(
+        self._checkAndCreateNewsItem(
             {
                 "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!",
+            },
+            False,
         )
 
         # 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(
+        self._checkAndCreateNewsItem(
             {
                 "display_if_installed": [
                     "net-misc/openssh",
@@ -261,16 +268,11 @@ class NewsItemTestCase(TestCase):
                 ]
             }
         )
-        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 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(
+        self._checkAndCreateNewsItem(
             {
                 "display_if_installed": [
                     "net-misc/openssh",
@@ -278,48 +280,33 @@ class NewsItemTestCase(TestCase):
                 ]
             }
         )
-        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(
+        self._checkAndCreateNewsItem(
             {
                 "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!",
+            },
+            False,
         )
 
         # What if we have a newer version of the listed package which
         # should match the constraint?
-        item = self._createNewsItem(
+        self._checkAndCreateNewsItem(
             {
                 "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(
+        self._checkAndCreateNewsItem(
             {
                 "display_if_installed": [
                     ">=net-misc/openssh-9.2_p1",
@@ -327,73 +314,39 @@ class NewsItemTestCase(TestCase):
                 ]
             }
         )
-        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())
-        self.assertTrue(
-            item.isRelevant(self.vardb, self.settings, self.profile),
-            msg=f"Expected {item} to be relevant, but it was not!",
-        )
+        self._checkAndCreateNewsItem({"display_if_keyword": [self.keywords]})
 
         # 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!",
-        )
+        self._checkAndCreateNewsItem({"display_if_keyword": ["fake-keyword"]}, False)
 
         # What if several keywords are listed and we match one of them?
-        item = self._createNewsItem(
+        self._checkAndCreateNewsItem(
             {"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!",
-        )
+        self._checkAndCreateNewsItem({"display_if_keyword": ["amd64", "~hppa"]}, False)
 
         # What if the ~keyword (testing) keyword is listed but we're keyword (stable)?
-        item = self._createNewsItem(
+        self._checkAndCreateNewsItem(
             {
                 "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!",
+            },
+            False,
         )
 
         # What if the stable keyword is listed but we're ~keyword (testing)?
-        item = self._createNewsItem(
+        self._checkAndCreateNewsItem(
             {
                 "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
@@ -403,62 +356,44 @@ class NewsItemTestCase(TestCase):
         # 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(
+        self._checkAndCreateNewsItem(
             {
                 "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!",
+                "display_if_installed": ["sys-apps/portage"],
+            }
         )
 
         # What if there's a Display-If-Keyword that matches and a
         # Display-If-Installed which doesn't?
-        item = self._createNewsItem(
+        self._checkAndCreateNewsItem(
             {
                 "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!",
+                "display_if_installed": ["sys-apps/i-do-not-exist"],
+            },
+            False,
         )
 
         # 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(
+        self._checkAndCreateNewsItem(
             {
                 "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(
+        self._checkAndCreateNewsItem(
             {
                 "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!",
+            },
+            False,
         )


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

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

Thread overview: 2+ 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/, lib/portage/dbapi/ Sam James
  -- strict thread matches above, loose matches on Subject: below --
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