Switch to src layout and move tests to their own directory

masklinn · web-flow · commit 827347722bfb · 2023-04-06T21:16:26.000+02:00
- src/ layout seems to be the modern standard, and avoids e.g. false-positive success issues (where tests get run agains the source instead of the packaged library, and thus hide packaging issues): https://hynek.me/articles/testing-packaging/ - remove use of unittest entirely (switch everything to pytest, not just runner), also move doctesting to pytest - moving tests out avoids packaging them, and mucking up the source dir with more test files in the future - replace old test command by an invocation of tox The only thing that's lost is `setup.py check`, but turns out: - it only checks that `long_description` is valid and we don't use that - it wasn't being run on CI - invoking `setup.py` directly is getting deprecated
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -70,7 +70,7 @@ jobs:
     - name: Check results
       run: |
         # check that _regexes exists, and .eggs does not (== setuptools used our dependency)
-        test -e ua_parser/_regexes.py -a ! -e .eggs
+        test -e src/ua_parser/_regexes.py -a ! -e .eggs
 
   test:
     strategy:
@@ -95,6 +95,8 @@ jobs:
     steps:
     - name: Checkout working copy
       uses: actions/checkout@v3
+      with:
+        submodules: true
     - name: Set up Python ${{ matrix.python-version }}
       uses: actions/setup-python@v3
       with:
@@ -111,11 +113,12 @@ jobs:
         fi
         python -mpip install -r requirements_dev.txt
     - name: install package in environment
-      run: python setup.py develop
+      run: pip install .
     - name: run tests
-      # 
-      run: pytest -v -Werror -Wignore::ImportWarning
-    - name: run doctests
-      # pprint formatting was changed a lot in 3.5
       if: ${{ matrix.python-version != '2.7' }}
-      run: python -mdoctest README.rst
+      run: pytest -v -Werror -Wignore::ImportWarning --doctest-glob="*.rst"
+    - name: run tests
+      # pprint formatting was widely altered in 3.5, so can't run
+      # doctests before then
+      if: ${{ matrix.python-version == '2.7' }}
+      run: pytest -v -Werror -Wignore::ImportWarning
diff --git a/Makefile b/Makefile
@@ -1,20 +1,15 @@
 all: test
 
-test: clean
-	@mkdir -p tmp
-	@PYTHONPATH=tmp python setup.py develop -d tmp
-	@# run all tests
-	@PYTHONPATH=tmp python ua_parser/user_agent_parser_test.py
-	@# run a single test
-	@#PYTHONPATH=tmp python ua_parser/user_agent_parser_test.py ParseTest.testStringsDeviceBrandModel
+test:
+	tox
 
 clean:
 	@find . -name '*.pyc' -delete
 	@rm -rf tmp \
-	   ua_parser.egg-info \
+	   src/ua_parser.egg-info \
 	   dist \
 	   build \
-	   ua_parser/_regexes.py
+	   src/ua_parser/_regexes.py
 format:
 	@black .
 
diff --git a/setup.py b/setup.py
@@ -4,7 +4,7 @@
 from distutils import log
 from distutils.core import Command
 from distutils.command.build import build as _build
-from setuptools import setup
+from setuptools import setup, find_packages
 from setuptools.command.develop import develop as _develop
 from setuptools.command.sdist import sdist as _sdist
 from setuptools.command.install import install as _install
@@ -56,7 +56,7 @@ def finalize_options(self):
             )
 
         if self.inplace:
-            self.build_lib = "."
+            self.build_lib = "./src"
         else:
             self.set_undefined_options("build", ("build_lib", "build_lib"))
         if self.work_path is None:
@@ -164,7 +164,7 @@ def update_manifest(self):
         if not sdist.finalized:
             return
 
-        sdist.filelist.files.append("ua_parser/_regexes.py")
+        sdist.filelist.files.append("src/ua_parser/_regexes.py")
 
 
 class develop(_develop):
@@ -204,8 +204,8 @@ class sdist(_sdist):
     description="Python port of Browserscope's user agent parser",
     author="PBS",
     author_email="no-reply@pbs.org",
-    packages=["ua_parser"],
-    package_dir={"": "."},
+    packages=find_packages(where="src"),
+    package_dir={"": "src"},
     license="Apache 2.0",
     zip_safe=False,
     url="https://github.com/ua-parser/uap-python",
diff --git a/src/ua_parser/__init__.py b/src/ua_parser/__init__.py
diff --git a/src/ua_parser/user_agent_parser.py b/src/ua_parser/user_agent_parser.py
diff --git a/tests/test_legacy.py b/tests/test_legacy.py
@@ -14,10 +14,6 @@
 
 
 """User Agent Parser Unit Tests.
-Run:
-# python -m user_agent_parser_test  (runs all the tests, takes awhile)
-or like:
-# python -m user_agent_parser_test ParseTest.testBrowserscopeStrings
 """
 
 
@@ -30,8 +26,9 @@
 import platform
 import re
 import sys
-import unittest
 import warnings
+
+import pytest
 import yaml
 
 if platform.python_implementation() == "PyPy":
@@ -54,7 +51,7 @@
 )
 
 
-class ParseTest(unittest.TestCase):
+class ParseTest:
     def testBrowserscopeStrings(self):
         self.runUserAgentTestsFromYAML(
             os.path.join(TEST_RESOURCES_DIR, "tests/test_ua.yaml")
@@ -112,12 +109,8 @@ def testParseAll(self):
         }
 
         result = user_agent_parser.Parse(user_agent_string)
-        self.assertEqual(
-            result,
-            expected,
-            "UA: {0}\n expected<{1}> != actual<{2}>".format(
-                user_agent_string, expected, result
-            ),
+        assert result == expected, "UA: {0}\n expected<{1}> != actual<{2}>".format(
+            user_agent_string, expected, result
         )
 
     # Run a set of test cases from a YAML file
@@ -140,26 +133,22 @@ def runUserAgentTestsFromYAML(self, file_name):
 
             result = {}
             result = user_agent_parser.ParseUserAgent(user_agent_string)
-            self.assertEqual(
-                result,
-                expected,
-                "UA: {0}\n expected<{1}, {2}, {3}, {4}> != actual<{5}, {6}, {7}, {8}>".format(
-                    user_agent_string,
-                    expected["family"],
-                    expected["major"],
-                    expected["minor"],
-                    expected["patch"],
-                    result["family"],
-                    result["major"],
-                    result["minor"],
-                    result["patch"],
-                ),
-            )
-            self.assertLessEqual(
-                len(user_agent_parser._PARSE_CACHE),
-                user_agent_parser.MAX_CACHE_SIZE,
-                "verify that the cache size never exceeds the configured setting",
+            assert (
+                result == expected
+            ), "UA: {0}\n expected<{1}, {2}, {3}, {4}> != actual<{5}, {6}, {7}, {8}>".format(
+                user_agent_string,
+                expected["family"],
+                expected["major"],
+                expected["minor"],
+                expected["patch"],
+                result["family"],
+                result["major"],
+                result["minor"],
+                result["patch"],
             )
+            assert (
+                len(user_agent_parser._PARSE_CACHE) <= user_agent_parser.MAX_CACHE_SIZE
+            ), "verify that the cache size never exceeds the configured setting"
 
     def runOSTestsFromYAML(self, file_name):
         yamlFile = open(os.path.join(TEST_RESOURCES_DIR, file_name))
@@ -180,22 +169,20 @@ def runOSTestsFromYAML(self, file_name):
             }
 
             result = user_agent_parser.ParseOS(user_agent_string)
-            self.assertEqual(
-                result,
-                expected,
-                "UA: {0}\n expected<{1} {2} {3} {4} {5}> != actual<{6} {7} {8} {9} {10}>".format(
-                    user_agent_string,
-                    expected["family"],
-                    expected["major"],
-                    expected["minor"],
-                    expected["patch"],
-                    expected["patch_minor"],
-                    result["family"],
-                    result["major"],
-                    result["minor"],
-                    result["patch"],
-                    result["patch_minor"],
-                ),
+            assert (
+                result == expected
+            ), "UA: {0}\n expected<{1} {2} {3} {4} {5}> != actual<{6} {7} {8} {9} {10}>".format(
+                user_agent_string,
+                expected["family"],
+                expected["major"],
+                expected["minor"],
+                expected["patch"],
+                expected["patch_minor"],
+                result["family"],
+                result["major"],
+                result["minor"],
+                result["patch"],
+                result["patch_minor"],
             )
 
     def runDeviceTestsFromYAML(self, file_name):
@@ -215,35 +202,33 @@ def runDeviceTestsFromYAML(self, file_name):
             }
 
             result = user_agent_parser.ParseDevice(user_agent_string)
-            self.assertEqual(
-                result,
-                expected,
-                "UA: {0}\n expected<{1} {2} {3}> != actual<{4} {5} {6}>".format(
-                    user_agent_string,
-                    expected["family"],
-                    expected["brand"],
-                    expected["model"],
-                    result["family"],
-                    result["brand"],
-                    result["model"],
-                ),
+            assert (
+                result == expected
+            ), "UA: {0}\n expected<{1} {2} {3}> != actual<{4} {5} {6}>".format(
+                user_agent_string,
+                expected["family"],
+                expected["brand"],
+                expected["model"],
+                result["family"],
+                result["brand"],
+                result["model"],
             )
 
 
-class GetFiltersTest(unittest.TestCase):
+class GetFiltersTest:
     def testGetFiltersNoMatchesGiveEmptyDict(self):
         user_agent_string = "foo"
         filters = user_agent_parser.GetFilters(
             user_agent_string, js_user_agent_string=None
         )
-        self.assertEqual({}, filters)
+        assert {} == filters
 
     def testGetFiltersJsUaPassedThrough(self):
         user_agent_string = "foo"
         filters = user_agent_parser.GetFilters(
             user_agent_string, js_user_agent_string="bar"
         )
-        self.assertEqual({"js_user_agent_string": "bar"}, filters)
+        assert {"js_user_agent_string": "bar"} == filters
 
     def testGetFiltersJsUserAgentFamilyAndVersions(self):
         user_agent_string = (
@@ -254,38 +239,32 @@ def testGetFiltersJsUserAgentFamilyAndVersions(self):
         filters = user_agent_parser.GetFilters(
             user_agent_string, js_user_agent_string="bar", js_user_agent_family="foo"
         )
-        self.assertEqual(
-            {"js_user_agent_string": "bar", "js_user_agent_family": "foo"}, filters
-        )
+        assert {"js_user_agent_string": "bar", "js_user_agent_family": "foo"} == filters
 
 
-class TestDeprecationWarnings(unittest.TestCase):
+class TestDeprecationWarnings:
     def setUp(self):
         """In Python 2.7, catch_warnings apparently does not do anything if
         the warning category is not active, whereas in 3(.6 and up) it
         seems to work out of the box.
         """
-        super(TestDeprecationWarnings, self).setUp()
         warnings.simplefilter("always", DeprecationWarning)
 
     def tearDown(self):
         # not ideal as it discards all other warnings updates from the
         # process, should really copy the contents of
         # `warnings.filters`, then reset-it.
         warnings.resetwarnings()
-        super(TestDeprecationWarnings, self).tearDown()
 
     def test_parser_deprecation(self):
-        with warnings.catch_warnings(record=True) as ws:
+        with pytest.warns(DeprecationWarning) as ws:
             user_agent_parser.ParseWithJSOverrides("")
-        self.assertEqual(len(ws), 1)
-        self.assertEqual(ws[0].category, DeprecationWarning)
+        assert len(ws) == 1
 
     def test_printer_deprecation(self):
-        with warnings.catch_warnings(record=True) as ws:
+        with pytest.warns(DeprecationWarning) as ws:
             user_agent_parser.Pretty("")
-        self.assertEqual(len(ws), 1)
-        self.assertEqual(ws[0].category, DeprecationWarning)
+        assert len(ws) == 1
 
     def test_js_bits_deprecation(self):
         for parser, count in [
@@ -295,33 +274,28 @@ def test_js_bits_deprecation(self):
             (user_agent_parser.ParseDevice, 1),
         ]:
             user_agent_parser._PARSE_CACHE.clear()
-            with warnings.catch_warnings(record=True) as ws:
+            with pytest.warns(DeprecationWarning) as ws:
                 parser("some random thing", js_attribute=True)
-            self.assertEqual(len(ws), count)
-            for w in ws:
-                self.assertEqual(w.category, DeprecationWarning)
+            assert len(ws) == count
 
 
-class ErrTest(unittest.TestCase):
-    @unittest.skipIf(
-        sys.version_info < (3,), "bytes and str are not differentiated in P2"
+class ErrTest:
+    @pytest.mark.skipif(
+        sys.version_info < (3,),
+        reason="bytes and str are not differentiated in P2",
     )
     def test_bytes(self):
-        with self.assertRaises(TypeError):
+        with pytest.raises(TypeError):
             user_agent_parser.Parse(b"")
 
     def test_int(self):
-        with self.assertRaises(TypeError):
+        with pytest.raises(TypeError):
             user_agent_parser.Parse(0)
 
     def test_list(self):
-        with self.assertRaises(TypeError):
+        with pytest.raises(TypeError):
             user_agent_parser.Parse([])
 
     def test_tuple(self):
-        with self.assertRaises(TypeError):
+        with pytest.raises(TypeError):
             user_agent_parser.Parse(())
-
-
-if __name__ == "__main__":
-    unittest.main()
diff --git a/tox.ini b/tox.ini
@@ -8,18 +8,11 @@ skipsdist = True
 usedevelop = True
 deps = -rrequirements_dev.txt
 commands =
-    pytest -Werror {posargs}
-    python -mdoctest README.rst
+    pytest -Werror --doctest-glob="*.rst" {posargs}
 
 [testenv:py27]
 # no doctesting in 2.7 because of formatting divergences
-commands = pytest {posargs}
-
-[testenv:docs]
-skip_install = True
-deps = docutils
-       Pygments
-commands = python setup.py check -s --restructuredtext --metadata
+commands = pytest -Werror {posargs}
 
 [testenv:flake8]
 skip_install = True
@@ -33,4 +26,4 @@ commands = black --check --diff .
 
 [flake8]
 max_line_length = 88
-filename = ua_parser/
+filename = src/ua_parser/
