Merge pull request #35 from bcdev/forman-improve_coverage

Further improve coverage

Author: forman

docs/todo.md

@@ -14,15 +14,17 @@
 ## Desired
 
 - project logo
-- add some more tests so we reach 99% coverage
 - apply rule op args/kwargs validation schema 
 - provide core rule that checks for configurable list of std attributes
 - measure time it takes to open a dataset and pass time into rule context 
   so we can write a configurable rule that checks the opening time
-- allow outputting suggestions, if any, that emitted by some rules
+- allow outputting suggestions, if any, that are emitted by some rules
+- enhance styling of `Result` representation in Jupyter notebooks
+  (check if we can expand/collapse messages with suggestions)
 
 ## Nice to have
 
+- add some more tests so we reach 100% coverage
 - support `autofix` feature
 - support `md` (markdown) output format
 - support formatter op args/kwargs and apply validation schema

tests/formatters/test_simple.py

@@ -7,10 +7,10 @@
 
 
 class SimpleTest(TestCase):
-    results = [
+    errors_and_warnings = [
         Result.new(
             Config(),
-            file_path="test.nc",
+            file_path="test1.nc",
             messages=[
                 Message(message="what", rule_id="rule-1", severity=2),
                 Message(message="is", fatal=True),
@@ -19,29 +19,65 @@ class SimpleTest(TestCase):
         )
     ]
 
+    warnings_only = [
+        Result.new(
+            Config(),
+            file_path="test2.nc",
+            messages=[
+                Message(message="what", rule_id="rule-1", severity=1),
+                Message(message="happened?", rule_id="rule-2", severity=1),
+            ],
+        )
+    ]
+
     def test_no_color(self):
         formatter = Simple(styled=False)
         text = formatter.format(
             context=get_context(),
-            results=self.results,
+            results=self.errors_and_warnings,
+        )
+        self.assert_output_1_ok(text)
+        self.assertNotIn("\033]", text)
+
+        formatter = Simple(styled=False)
+        text = formatter.format(
+            context=get_context(),
+            results=self.warnings_only,
         )
-        self.assert_output_ok(text)
+        self.assert_output_2_ok(text)
         self.assertNotIn("\033]", text)
 
     def test_color(self):
         formatter = Simple(styled=True)
         text = formatter.format(
             context=get_context(),
-            results=self.results,
+            results=self.errors_and_warnings,
+        )
+        self.assert_output_1_ok(text)
+        self.assertIn("\033]", text)
+
+        formatter = Simple(styled=True)
+        text = formatter.format(
+            context=get_context(),
+            results=self.warnings_only,
         )
-        self.assert_output_ok(text)
+        self.assert_output_2_ok(text)
         self.assertIn("\033]", text)
 
-    def assert_output_ok(self, text):
+    def assert_output_1_ok(self, text):
         self.assertIsInstance(text, str)
-        self.assertIn("test.nc", text)
+        self.assertIn("test1.nc", text)
         self.assertIn("happening?", text)
         self.assertIn("error", text)
         self.assertIn("warn", text)
         self.assertIn("rule-1", text)
         self.assertIn("rule-2", text)
+
+    def assert_output_2_ok(self, text):
+        self.assertIsInstance(text, str)
+        self.assertIn("test2.nc", text)
+        self.assertIn("happened?", text)
+        self.assertNotIn("error", text)
+        self.assertIn("warn", text)
+        self.assertIn("rule-1", text)
+        self.assertIn("rule-2", text)

tests/test_config.py

@@ -1,14 +1,24 @@
+from typing import Any
 from unittest import TestCase
 
 import pytest
+import xarray as xr
 
-from xrlint.config import Config, ConfigList
-from xrlint.rule import RuleConfig
+from xrlint.config import Config, ConfigList, get_core_config
+from xrlint.constants import CORE_PLUGIN_NAME
+from xrlint.plugin import Plugin, new_plugin
+from xrlint.processor import define_processor, ProcessorOp
+from xrlint.result import Message
+from xrlint.rule import RuleConfig, Rule
 from xrlint.util.filefilter import FileFilter
 
 
 # noinspection PyMethodMayBeStatic
 class ConfigTest(TestCase):
+    def test_class_props(self):
+        self.assertEqual("config", Config.value_name())
+        self.assertEqual("Config | dict | None", Config.value_type_name())
+
     def test_defaults(self):
         config = Config()
         self.assertEqual(None, config.name)
@@ -20,6 +30,50 @@ def test_defaults(self):
         self.assertEqual(None, config.plugins)
         self.assertEqual(None, config.rules)
 
+    def test_get_plugin(self):
+        config = get_core_config()
+        plugin = config.get_plugin(CORE_PLUGIN_NAME)
+        self.assertIsInstance(plugin, Plugin)
+
+        with pytest.raises(ValueError, match="unknown plugin 'xcube'"):
+            config.get_plugin("xcube")
+
+    def test_get_rule(self):
+        config = get_core_config()
+        rule = config.get_rule("flags")
+        self.assertIsInstance(rule, Rule)
+
+        with pytest.raises(ValueError, match="unknown rule 'foo'"):
+            config.get_rule("foo")
+
+    def test_get_processor_op(self):
+        class MyProc(ProcessorOp):
+            def preprocess(
+                self, file_path: str, opener_options: dict[str, Any]
+            ) -> list[tuple[xr.Dataset, str]]:
+                pass
+
+            def postprocess(
+                self, messages: list[list[Message]], file_path: str
+            ) -> list[Message]:
+                pass
+
+        processor = define_processor("myproc", op_class=MyProc)
+        config = Config(
+            plugins=dict(
+                myplugin=new_plugin("myplugin", processors=dict(myproc=processor))
+            )
+        )
+
+        processor_op = config.get_processor_op(MyProc())
+        self.assertIsInstance(processor_op, MyProc)
+
+        processor_op = config.get_processor_op("myplugin/myproc")
+        self.assertIsInstance(processor_op, MyProc)
+
+        with pytest.raises(ValueError, match="unknown processor 'myplugin/myproc2'"):
+            config.get_processor_op("myplugin/myproc2")
+
     def test_from_value_ok(self):
         self.assertEqual(Config(), Config.from_value(None))
         self.assertEqual(Config(), Config.from_value({}))
@@ -167,7 +221,7 @@ def test_compute_config(self):
 
         config_list = ConfigList(
             [
-                Config(settings={"a": 1, "b": 1}),
+                Config(ignores=["**/*.yaml"], settings={"a": 1, "b": 1}),
                 Config(files=["**/datacubes/*.zarr"], settings={"b": 2}),
                 Config(files=["**/*.txt"], settings={"a": 2}),
             ]
@@ -185,6 +239,12 @@ def test_compute_config(self):
             config_list.compute_config(file_path),
         )
 
+        file_path = "s3://wq-services/datacubes/config.yaml"
+        self.assertEqual(
+            None,
+            config_list.compute_config(file_path),
+        )
+
     def test_split_global_filter(self):
         config_list = ConfigList(
             [

tests/test_operation.py

@@ -32,7 +32,7 @@ def op_base_class(cls) -> Type[ThingOp]:
         return ThingOp
 
     @classmethod
-    def op_name(cls) -> str:
+    def value_name(cls) -> str:
         return "thing"
 
     @classmethod
@@ -62,10 +62,9 @@ class MyThingOp3(ThingOp):
 
 
 class OperationTest(TestCase):
-    def test_defaults(self):
+    def test_class_props(self):
         self.assertEqual(OperationMeta, Operation.meta_class())
         self.assertEqual(type, Operation.op_base_class())
-        self.assertEqual("operation", Operation.op_name())
         self.assertEqual("export_operation", Operation.op_import_attr_name())
         self.assertEqual("operation", Operation.value_name())
         self.assertEqual(

tests/test_plugin.py

@@ -10,6 +10,10 @@
 
 
 class PluginTest(TestCase):
+    def test_class_props(self):
+        self.assertEqual("plugin", Plugin.value_name())
+        self.assertEqual("Plugin | dict | str", Plugin.value_type_name())
+
     def test_new_plugin(self):
         plugin = new_plugin(name="hello", version="2.4.5")
         self.assertEqual(Plugin(meta=PluginMeta(name="hello", version="2.4.5")), plugin)
@@ -53,6 +57,10 @@ class MyRule2(RuleOp):
 
 
 class PluginMetaTest(TestCase):
+    def test_class_props(self):
+        self.assertEqual("plugin_meta", PluginMeta.value_name())
+        self.assertEqual("PluginMeta | dict", PluginMeta.value_type_name())
+
     def test_from_value(self):
         self.assertEqual(
             PluginMeta(name="p", ref="a.b.c:p"),

tests/test_processor.py

@@ -9,7 +9,19 @@
 from xrlint.result import Message
 
 
+class ProcessorMetaTest(TestCase):
+    def test_class_props(self):
+        self.assertEqual("processor_meta", ProcessorMeta.value_name())
+        self.assertEqual("ProcessorMeta | dict", ProcessorMeta.value_type_name())
+
+
 class ProcessorTest(TestCase):
+    def test_class_props(self):
+        self.assertEqual("processor", Processor.value_name())
+        self.assertEqual(
+            "Processor | Type[ProcessorOp] | dict | str", Processor.value_type_name()
+        )
+
     def test_define_processor(self):
         registry = {}
 

tests/test_rule.py

@@ -19,6 +19,12 @@ def export_rule():
 
 
 class RuleTest(TestCase):
+    def test_class_props(self):
+        self.assertIs(RuleMeta, Rule.meta_class())
+        self.assertIs(RuleOp, Rule.op_base_class())
+        self.assertEqual("rule", Rule.value_name())
+        self.assertEqual("Rule | Type[RuleOp] | dict | str", Rule.value_type_name())
+
     def test_from_value_ok_rule(self):
         rule = export_rule()
         rule2 = Rule.from_value(rule)
@@ -72,6 +78,10 @@ class MyRule3(RuleOp):
 
 
 class RuleMetaTest(unittest.TestCase):
+    def test_class_props(self):
+        self.assertEqual("rule_meta", RuleMeta.value_name())
+        self.assertEqual("RuleMeta | dict", RuleMeta.value_type_name())
+
     def test_from_value(self):
         rule_meta = RuleMeta.from_value(
             {
@@ -133,6 +143,10 @@ def test_fail(self):
 
 
 class RuleConfigTest(TestCase):
+    def test_class_props(self):
+        self.assertEqual("rule_config", RuleConfig.value_name())
+        self.assertEqual("int | str | list", RuleConfig.value_type_name())
+
     def test_defaults(self):
         rule_config = RuleConfig(1)
         self.assertEqual(1, rule_config.severity)
@@ -147,6 +161,10 @@ def test_from_value_ok(self):
         self.assertEqual(RuleConfig(1), RuleConfig.from_value("warn"))
         self.assertEqual(RuleConfig(2), RuleConfig.from_value("error"))
         self.assertEqual(RuleConfig(2), RuleConfig.from_value(["error"]))
+        # YAML "on"/"off" literals
+        self.assertEqual(RuleConfig(0), RuleConfig.from_value(False))
+        self.assertEqual(RuleConfig(1), RuleConfig.from_value(True))
+
         self.assertEqual(
             RuleConfig(1, ("never",)), RuleConfig.from_value(["warn", "never"])
         )
@@ -175,17 +193,19 @@ def test_from_value_ok(self):
         )
 
     # noinspection PyMethodMayBeStatic
-    def test_from_value_fails(self):
+    def test_from_value_fail(self):
         with pytest.raises(
             TypeError,
-            match="rule configuration must be of type int|str|tuple|list, but got None",
+            match=r"rule_config must be of type int \| str \| list, but got None",
         ):
             RuleConfig.from_value(None)
+
         with pytest.raises(
             ValueError,
             match="severity must be one of 'error', 'warn', 'off', 2, 1, 0, but got 4",
         ):
             RuleConfig.from_value(4)
+
         with pytest.raises(
             ValueError,
             match=(
@@ -194,3 +214,9 @@ def test_from_value_fails(self):
             ),
         ):
             RuleConfig.from_value("debug")
+
+        with pytest.raises(
+            ValueError,
+            match="rule_config must not be empty",
+        ):
+            RuleConfig.from_value([])

tests/util/test_filefilter.py

@@ -62,9 +62,26 @@ def test_accept(self):
         self.assertEqual(True, file_filter.accept("test-1.zarr"))
         self.assertEqual(True, file_filter.accept("test-2.zarr"))
 
+        # negated ignores:
         file_filter = FileFilter.from_patterns(
             ["**/*.zarr"], ["**/temp/*", "!**/temp/x.zarr"]
         )
         self.assertEqual(True, file_filter.accept("./test.zarr"))
         self.assertEqual(True, file_filter.accept("./temp/x.zarr"))
         self.assertEqual(False, file_filter.accept("./temp/y.zarr"))
+
+        # negated ignores:
+        file_filter = FileFilter.from_patterns([], ["**/temp/*", "!**/temp/*.zarr"])
+        self.assertEqual(True, file_filter.accept("./ds.nc"))
+        self.assertEqual(True, file_filter.accept("./temp/ds.zarr"))
+        self.assertEqual(True, file_filter.accept("./temp/test/ds.zarr"))
+        self.assertEqual(False, file_filter.accept("./temp/ds.nc"))
+        self.assertEqual(False, file_filter.accept("./temp/README.md"))
+
+        # just ignores:
+        file_filter = FileFilter.from_patterns([], ["**/*.json", "**/*.yaml"])
+        self.assertEqual(True, file_filter.accept("./test.nc"))
+        self.assertEqual(True, file_filter.accept("./temp/x.zarr"))
+        self.assertEqual(False, file_filter.accept("c.yaml"))
+        self.assertEqual(False, file_filter.accept("test/config.json"))
+        self.assertEqual(False, file_filter.accept("test/config.yaml"))