Merge pull request #3862 from mtmail/skip-all-zero-postcodes

Postcode sanetizer now skips values which are only zeros

docs/develop/Testing.md

@@ -52,6 +52,15 @@ To run the functional tests, do
 
     pytest test/bdd
 
+You can run a single feature file using expression matching:
+
+    pytest test/bdd -k osm2pgsql/import/entrances.feature
+
+This even works for running single tests by adding the line number of the
+scenario header like that:
+
+    pytest test/bdd -k 'osm2pgsql/import/entrances.feature and L4'
+
 The BDD tests create databases for the tests. You can set name of the databases
 through configuration variables in your `pytest.ini`:
 

src/nominatim_db/data/postcode_format.py

@@ -29,6 +29,9 @@ class CountryPostcodeMatcher:
         self.norm_pattern = re.compile(f'\\s*(?:{country_code.upper()}[ -]?)?({pc_pattern})\\s*')
         self.pattern = re.compile(pc_pattern)
 
+        # We want to exclude 0000, 00-000, 000 00 etc
+        self.zero_pattern = re.compile(r'^[0\- ]+$')
+
         self.output = config.get('output', r'\g<0>')
 
     def match(self, postcode: str) -> Optional[Match[str]]:
@@ -40,7 +43,10 @@ class CountryPostcodeMatcher:
         normalized = self.norm_pattern.fullmatch(postcode.upper())
 
         if normalized:
-            return self.pattern.fullmatch(normalized.group(1))
+            match = self.pattern.fullmatch(normalized.group(1))
+            if match and self.zero_pattern.match(match.string):
+                return None
+            return match
 
         return None
 

test/bdd/conftest.py

@@ -9,6 +9,7 @@ Fixtures for BDD test steps
 """
 import sys
 import json
+import re
 from pathlib import Path
 
 import psycopg
@@ -20,7 +21,8 @@ sys.path.insert(0, str(SRC_DIR / 'src'))
 
 import pytest
 from pytest_bdd.parsers import re as step_parse
-from pytest_bdd import given, when, then
+from pytest_bdd import given, when, then, scenario
+from pytest_bdd.feature import get_features
 
 pytest.register_assert_rewrite('utils')
 
@@ -373,3 +375,57 @@ def check_place_missing_lines(db_conn, table, osm_type, osm_id, osm_class):
 
     with db_conn.cursor() as cur:
         assert cur.execute(sql, params).fetchone()[0] == 0
+
+
+if pytest.version_tuple >= (8, 0, 0):
+    def pytest_pycollect_makemodule(module_path, parent):
+        return BddTestCollector.from_parent(parent, path=module_path)
+
+
+class BddTestCollector(pytest.Module):
+
+    def __init__(self, **kwargs):
+        super().__init__(**kwargs)
+
+    def collect(self):
+        for item in super().collect():
+            yield item
+
+        if hasattr(self.obj, 'PYTEST_BDD_SCENARIOS'):
+            for path in self.obj.PYTEST_BDD_SCENARIOS:
+                for feature in get_features([str(Path(self.path.parent, path).resolve())]):
+                    yield FeatureFile.from_parent(self,
+                                                  name=str(Path(path, feature.rel_filename)),
+                                                  path=Path(feature.filename),
+                                                  feature=feature)
+
+
+# borrowed from pytest-bdd: src/pytest_bdd/scenario.py
+def make_python_name(string: str) -> str:
+    """Make python attribute name out of a given string."""
+    string = re.sub(r"\W", "", string.replace(" ", "_"))
+    return re.sub(r"^\d+_*", "", string).lower()
+
+
+class FeatureFile(pytest.File):
+    class obj:
+        pass
+
+    def __init__(self, feature, **kwargs):
+        self.feature = feature
+        super().__init__(**kwargs)
+
+    def collect(self):
+        for sname, sobject in self.feature.scenarios.items():
+            class_name = f"L{sobject.line_number}"
+            test_name = "test_" + make_python_name(sname)
+
+            @scenario(self.feature.filename, sname)
+            def _test():
+                pass
+
+            tclass = type(class_name, (),
+                          {test_name: staticmethod(_test)})
+            setattr(self.obj, class_name, tclass)
+
+            yield pytest.Class.from_parent(self, name=class_name, obj=tclass)

test/bdd/test_api.py

@@ -15,7 +15,7 @@ import xml.etree.ElementTree as ET
 
 import pytest
 from pytest_bdd.parsers import re as step_parse
-from pytest_bdd import scenarios, when, given, then
+from pytest_bdd import when, given, then
 
 from nominatim_db import cli
 from nominatim_db.config import Configuration
@@ -150,4 +150,8 @@ def parse_api_json_response(api_response, fmt, num):
     return result
 
 
-scenarios('features/api')
+if pytest.version_tuple >= (8, 0, 0):
+    PYTEST_BDD_SCENARIOS = ['features/api']
+else:
+    from pytest_bdd import scenarios
+    scenarios('features/api')

test/bdd/test_db.py

@@ -15,7 +15,7 @@ import re
 import psycopg
 
 import pytest
-from pytest_bdd import scenarios, when, then, given
+from pytest_bdd import when, then, given
 from pytest_bdd.parsers import re as step_parse
 
 from utils.place_inserter import PlaceColumn
@@ -276,4 +276,8 @@ def then_check_interpolation_table_negative(db_conn, oid):
         assert cur.fetchone()[0] == 0
 
 
-scenarios('features/db')
+if pytest.version_tuple >= (8, 0, 0):
+    PYTEST_BDD_SCENARIOS = ['features/db']
+else:
+    from pytest_bdd import scenarios
+    scenarios('features/db')

test/bdd/test_osm2pgsql.py

@@ -11,7 +11,7 @@ import asyncio
 import random
 
 import pytest
-from pytest_bdd import scenarios, when, then, given
+from pytest_bdd import when, then, given
 from pytest_bdd.parsers import re as step_parse
 
 from nominatim_db import cli
@@ -106,4 +106,8 @@ def check_place_content(db_conn, datatable, node_grid, table, exact):
     check_table_content(db_conn, table, datatable, grid=node_grid, exact=bool(exact))
 
 
-scenarios('features/osm2pgsql')
+if pytest.version_tuple >= (8, 0, 0):
+    PYTEST_BDD_SCENARIOS = ['features/osm2pgsql']
+else:
+    from pytest_bdd import scenarios
+    scenarios('features/osm2pgsql')

test/python/tokenizer/sanitizers/test_clean_postcodes.py

@@ -237,3 +237,9 @@ def test_postcode_default_pattern_pass(sanitize, postcode):
 @pytest.mark.sanitizer_params(convert_to_address=False, default_pattern='[A-Z0-9- ]{3,12}')
 def test_postcode_default_pattern_fail(sanitize, postcode):
     assert sanitize(country='an', postcode=postcode) == []
+
+
+@pytest.mark.parametrize("postcode", ('00000', '00-000', 'PL-00000', 'PL 00-000'))
+@pytest.mark.sanitizer_params(convert_to_address=False)
+def test_postcode_zeros(sanitize, postcode):
+    assert sanitize(country='pl', postcode=postcode) == []