From bb1ff53fc1c0ac05bf722d1ce97c41dcff210ac5 Mon Sep 17 00:00:00 2001
From: yohplala <pierre.juillard@gmail.com>
Date: Fri, 1 Mar 2024 08:50:49 +0100
Subject: [PATCH] Adding test cases for exceptions.

---
 oups/aggstream/aggstream.py                 | 22 ++++---
 tests/test_aggstream/test_aggstream_init.py | 63 +++++++++++++++++----
 2 files changed, 63 insertions(+), 22 deletions(-)

diff --git a/oups/aggstream/aggstream.py b/oups/aggstream/aggstream.py
index 6f476af..a9910f4 100644
--- a/oups/aggstream/aggstream.py
+++ b/oups/aggstream/aggstream.py
@@ -1088,13 +1088,18 @@ def __init__(
             # Check 'filters' parameter is used.
             if filters is None:
                 raise ValueError(
-                    "not possible to use filter syntax for `keys`"
-                    " parameter without providing `filters`"
-                    " parameter as well.",
+                    "not possible to use filter syntax for `keys` parameter "
+                    "without providing `filters` parameter as well.",
                 )
             else:
                 # Check same filters id are both in 'keys' and 'filters'
                 # parameters.
+                if NO_FILTER_ID in filters and filters[NO_FILTER_ID] is not None:
+                    raise ValueError(
+                        f"not possible to use '{NO_FILTER_ID}' as key in "
+                        "`filters` parameter with a value different than "
+                        "`None`.",
+                    )
                 filt_filt_ids = set(filters)
                 filt_filt_ids.discard(NO_FILTER_ID)
                 if filt_filt_ids != (keys_filt_ids := set(keys)):
@@ -1313,15 +1318,8 @@ def agg(
 # Should we persist / store 'p_job' between 'AggStream.agg' execution?
 
 # Tests:
-
-# Test exceptions
-# - test case, test parameter value not in 'streamagg' nor in 'write' signature.
-# - test filter ids is same between "keys" parameter and "filters" parameter or raise error
-# - test filter syntax used but not 'filters' parameter
-# - test using default_filter_id in "filters" dict
-# - test with a filter defined in 'filters' parameter but not used in 'keys' parameter.
-# - do a test using only "no filter id" "_" and do not specify 'filters' parameter.
-# Other tests
+# - do a test using only "no filter id" "_" with another filter and check no filter
+#   is used indeed.
 # - in test case with snapshot: when snapshot is a TimeGrouper, make sure that stitching
 # works same as for bin: that empty snapshots are generated between 2 row groups.
 # - test case, test parameter value not in 'streamagg' nor in 'write' signature.
diff --git a/tests/test_aggstream/test_aggstream_init.py b/tests/test_aggstream/test_aggstream_init.py
index f3b24cc..3773170 100644
--- a/tests/test_aggstream/test_aggstream_init.py
+++ b/tests/test_aggstream/test_aggstream_init.py
@@ -50,6 +50,9 @@ class Indexer:
     dataset_ref: str
 
 
+key = Indexer("agg_res")
+
+
 @pytest.fixture
 def store(tmp_path):
     # Reuse pre-defined Indexer.
@@ -390,7 +393,6 @@ def test_exception_not_key_of_streamagg_results(store):
     ordered_on = "ts_order"
     val = range(1, len(ts) + 1)
     seed_pdf = pDataFrame({ordered_on: ts, "val": val})
-    key = Indexer("agg_res")
     store[key] = seed_pdf
     # Setup aggregation.
     bin_by = TimeGrouper(key=ordered_on, freq="1H", closed="left", label="left")
@@ -405,16 +407,57 @@ def test_exception_not_key_of_streamagg_results(store):
         )
 
 
-def test_exception_setup_no_bin_by(store):
+@pytest.mark.parametrize(
+    "other_parameters, exception_mess",
+    [
+        # 0 / No 'bin_by'
+        (
+            {"keys": {key: {"agg": {"out_spec": ("in_spec", FIRST)}}}},
+            "^'bin_by' parameter is missing",
+        ),
+        # 1 / Parameter not in 'AggStream.__init__' nor in 'write' signature.
+        # Because '**kwargs' is used in 'AggStream.__init__', test the
+        # implemented check.
+        (
+            {
+                "keys": {key: {"agg": {"out_spec": ("in_spec", FIRST)}}},
+                "my_invented_parameter": 0,
+            },
+            "^'my_invented_parameter' is neither",
+        ),
+        # 2 / Different filter ids between 'keys' and 'filters' parameters.
+        (
+            {
+                "keys": {"filter1": {key: {"agg": {"out_spec": ("in_spec", FIRST)}}}},
+                "filters": {"filter2": ["val", ">=", 0]},
+            },
+            "^not possible to have different lists of filter ids",
+        ),
+        # 3 / Filter syntax used in 'keys' parameter, without 'filters' parameter.
+        (
+            {
+                "keys": {"filter1": {key: {"agg": {"out_spec": ("in_spec", FIRST)}}}},
+            },
+            "^not possible to use filter syntax for `keys`",
+        ),
+        # 4 / Use "no filter" filter id in 'filters' to set a filter.
+        (
+            {
+                "keys": {NO_FILTER_ID: {key: {"agg": {"out_spec": ("in_spec", FIRST)}}}},
+                "filters": {NO_FILTER_ID: ["val", ">=", 0]},
+            },
+            f"^not possible to use '{NO_FILTER_ID}'",
+        ),
+    ],
+)
+def test_exceptions_Aggstream_parameters(store, other_parameters, exception_mess):
     ordered_on = "ts"
-    key = Indexer("agg_res")
-    keys_config = {key: {"agg": {"out_spec": ("in_spec", FIRST)}}}
+    conf = {
+        "store": store,
+        "ordered_on": ordered_on,
+    } | other_parameters
     # Test.
-    with pytest.raises(ValueError, match="^'bin_by' parameter is missing"):
+    with pytest.raises(ValueError, match=exception_mess):
         AggStream(
-            store=store,
-            keys=keys_config,
-            ordered_on=ordered_on,
-            agg=None,
-            post=None,
+            **conf,
         )