Repository: projectsyn/reclass-rs (language: python)

Issue:
Add support for merging a `Mapping` into another `Mapping`
In order to handle constant keys and overrides in a consistent fashion, we split out the `insert()` implementation into a private method which has extra flags which allow the caller to force a key to be constant or overriding.

We set these flags based on the contents of `const_keys` and `override_keys` in the Mapping that's getting merged into the target Mapping to correctly propagate constant keys and unprocessed overrides.

Note that this approach differs from Python Reclass, which tracks constantness and overrides in the Mapping values and only processes this information when a `ValueList` is flattened into a single value.

As far as we've been able to determine, our implementation should produce identical results to Python Reclass, with the possible exception of some combinations of multiple key prefixes for a single key.

<!--
Thank you for your pull request. Please provide a description above and
review the checklist below.

Contributors guide: ./CONTRIBUTING.md
-->

## Checklist
<!--
Remove items that do not apply. For completed items, change [ ] to [x].
-->

- [x] The PR has a meaningful title. The title will be used to auto generate the changelog
- [x] PR contains a single logical change (to build a better changelog).
- [x] Update the documentation.
- [x] Update tests.
- [x] Categorize the PR by setting a good title and adding one of the labels:
      `bug`, `enhancement`, `documentation`, `change`, `breaking`, `dependency`, `internal`
      as they show up in the changelog

<!--
NOTE: these things are not required to open a PR and can be done afterwards,
while the PR is open.
-->

Implement `ValueList` flattening
This PR introduces `Value::flattened()` which returns a copy of the Value on which it is called with any ValueLists processed and flattened into other Value variants. The merging logic for flattening a list of values is factored into a separate private method `Value::merge()` which performs the recursive merge of the value list elements.

`Value::merge()` calls `Mapping::merge()` to merge mappings which are stored in a ValueList. `Mapping::merge()` also calls `flattened()` again to ensure that any new ValueLists which are created by `Mapping::merge()` end up flattened in the final result of the original `flattened()` call.

Finally, the PR also introduces `Value::flatten()` which flattens a Value in-place, by calling `Value::flattened()` and replacing the original value with the result of `flattened()`.

We also move the `types::value` tests to separate source files, since the original `src/types/value.rs` got rather larger (> 1000LoC). The files `src/types/value/value_tests.rs` and `src/types/value/value_as_py_obj_tests.rs` are identical to the previous inline `value_tests` and `value_as_py_obj_tests` modules (see 8f25144572ae02e5d83bb1718b2276fd36ac58fa).

The file `src/types/value_flattened_tests.rs` contains the new tests for `Value::flattened()` and `Value::flatten()`.

## Checklist
<!--
Remove items that do not apply. For completed items, change [ ] to [x].
-->

- [x] The PR has a meaningful title. The title will be used to auto generate the changelog
- [x] PR contains a single logical change (to build a better changelog).
- [x] Update the documentation.
- [x] Update tests.
- [x] Categorize the PR by setting a good title and adding one of the labels:
      `bug`, `enhancement`, `documentation`, `change`, `breaking`, `dependency`, `internal`
      as they show up in the changelog


<!--
NOTE: these things are not required to open a PR and can be done afterwards,
while the PR is open.
-->



Discussion / hints:

Fix patch:
diff --git a/Cargo.toml b/Cargo.toml
index e1ba588..e1976a9 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -18,6 +18,7 @@ nom = "7.1.3"
 pyo3 = { version = "0.19.2", features = ["chrono"] }
 serde = { version = "1.0.188", features = ["derive"] }
 serde_yaml = "0.9.25"
+walkdir = "2.4.0"
 yaml-merge-keys = { version = "0.6.0", features = ["serde_yaml"] }
 
 [dev-dependencies]
diff --git a/src/lib.rs b/src/lib.rs
index 826279d..76cbb6f 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -11,11 +11,17 @@ mod node;
 mod refs;
 pub mod types;
 
+use anyhow::{anyhow, Result};
 use pyo3::exceptions::PyValueError;
 use pyo3::prelude::*;
+use std::collections::HashMap;
+use std::path::{Path, PathBuf, MAIN_SEPARATOR};
+use walkdir::WalkDir;
 
 use crate::node::{Node, NodeInfo, NodeInfoMeta};
 
+const SUPPORTED_YAML_EXTS: [&str; 2] = ["yml", "yaml"];
+
 /// This struct holds configuration fields for various library behaviors
 #[pyclass]
 #[derive(Clone, Debug)]
@@ -29,18 +35,85 @@ pub struct Reclass {
     /// Whether to ignore included classes which don't exist (yet)
     #[pyo3(get)]
     pub ignore_class_notfound: bool,
+    classes: HashMap<String, PathBuf>,
+}
+
+impl Reclass {
+    /// Discover all classes in `r.classes_path` and store the resulting list in `r.known_classes`.
+    ///
+    /// This method will raise an error if multiple classes which resolve to the same absolute
+    /// class name exist (e.g. classes `foo..bar.yml` and `foo/.bar.yml` are both included as
+    /// `foo..bar`).
+    fn discover_classes(&mut self) -> Result<()> {
+        fn stringify(p: &Path) -> Result<&str> {
+            p.to_str()
+                .ok_or(anyhow!("Failed to convert {} to string", p.display()))
+        }
+        let class_root = PathBuf::from(&self.classes_path).canonicalize()?;
+
+        for entry in WalkDir::new(&self.classes_path) {
+            let entry = entry?;
+            let ext = if let Some(ext) = entry.path().extension() {
+                ext.to_str()
+            } else {
+                None
+            };
+            if ext.is_some() && SUPPORTED_YAML_EXTS.contains(&ext.unwrap()) {
+                // it's a class, process it
+                let abspath = entry.path().canonicalize()?;
+                let relpath = abspath.strip_prefix(&class_root)?;
+                let cls = relpath
+                    .with_extension("")
+                    .to_str()
+                    .ok_or(anyhow!(
+                        "Failed to canonicalize class {}",
+                        entry.path().display()
+                    ))?
+                    .replace(MAIN_SEPARATOR, ".");
+                if let Some(prev) = self.classes.get(&cls) {
+                    let mut previnv = PathBuf::from(&self.classes_path);
+                    previnv.push(prev);
+                    let prev = stringify(&previnv)?;
+                    let mut pathinv = PathBuf::from(&self.classes_path);
+                    pathinv.push(relpath);
+                    let relpath = stringify(&pathinv)?;
+                    // Ensure error message is stable without having to sort the directory walk
+                    // iterator.
+                    let (first, second) = if prev.cmp(relpath).is_lt() {
+                        (prev, relpath)
+                    } else {
+                        (relpath, prev)
+                    };
+                    return Err(anyhow!(
+                        "Definition of class '{cls}' in '{first}' collides with definition in '{second}'. \
+                        Classes can only be defined once per inventory."
+                    ));
+                }
+                self.classes.insert(cls, relpath.to_path_buf());
+            }
+        }
+        Ok(())
+    }
 }
 
 #[pymethods]
 impl Reclass {
     #[new]
     #[pyo3(signature = (nodes_path="./inventory/nodes", classes_path="./inventory/classes", ignore_class_notfound=false))]
-    pub fn new(nodes_path: &str, classes_path: &str, ignore_class_notfound: bool) -> Self {
-        Self {
+    pub fn new(
+        nodes_path: &str,
+        classes_path: &str,
+        ignore_class_notfound: bool,
+    ) -> PyResult<Self> {
+        let mut r = Self {
             nodes_path: nodes_path.to_owned(),
             classes_path: classes_path.to_owned(),
             ignore_class_notfound,
-        }
+            classes: HashMap::new(),
+        };
+        r.discover_classes()
+            .map_err(|e| PyValueError::new_err(format!("Error while discovering classes: {e}")))?;
+        Ok(r)
     }
 
     fn __repr__(&self) -> String {
@@ -49,15 +122,18 @@ impl Reclass {
 
     /// Returns the rendered data for the node with the provided name if it exists
     pub fn nodeinfo(&self, nodename: &str) -> PyResult<NodeInfo> {
-        let n = Node::parse(self, nodename)
+        let mut n = Node::parse(self, nodename)
             .map_err(|e| PyValueError::new_err(format!("Error while parsing {nodename}: {e}")))?;
+        n.render(self)
+            .map_err(|e| PyValueError::new_err(format!("Error while rendering {nodename}: {e}")))?;
+
         Ok(n.into())
     }
 }
 
 impl Default for Reclass {
     fn default() -> Self {
-        Self::new("./inventory/nodes", "./inventory/classes", false)
+        Self::new("./inventory/nodes", "./inventory/classes", false).unwrap()
     }
 }
 
@@ -77,17 +153,29 @@ mod tests {
 
     #[test]
     fn test_reclass_new() {
-        let n = Reclass::new("./inventory/nodes", "./inventory/classes", false);
-        assert_eq!(n.nodes_path, "./inventory/nodes");
-        assert_eq!(n.classes_path, "./inventory/classes");
+        let n = Reclass::new(
+            "./tests/inventory/nodes",
+            "./tests/inventory/classes",
+            false,
+        )
+        .unwrap();
+        assert_eq!(n.nodes_path, "./tests/inventory/nodes");
+        assert_eq!(n.classes_path, "./tests/inventory/classes");
         assert_eq!(n.ignore_class_notfound, false);
     }
 
     #[test]
-    fn test_reclass_default() {
-        let d = Reclass::default();
-        assert_eq!(d.nodes_path, "./inventory/nodes");
-        assert_eq!(d.classes_path, "./inventory/classes");
-        assert_eq!(d.ignore_class_notfound, false);
+    #[should_panic(expected = "Error while discovering classes: \
+        Definition of class 'foo.bar' in './tests/broken-inventory/classes/foo.bar.yml' \
+        collides with definition in './tests/broken-inventory/classes/foo/bar.yml'. \
+        Classes can only be defined once per inventory.")]
+    fn test_reclass_discover_classes() {
+        pyo3::prepare_freethreaded_python();
+        Reclass::new(
+            "./tests/broken-inventory/nodes",
+            "./tests/broken-inventory/classes",
+            false,
+        )
+        .unwrap();
     }
 }
diff --git a/src/node/mod.rs b/src/node/mod.rs
index 7d80e5b..9ec5066 100644
--- a/src/node/mod.rs
+++ b/src/node/mod.rs
@@ -1,4 +1,4 @@
-use anyhow::{anyhow, Result};
+use anyhow::{anyhow, Context, Result};
 use serde::Deserialize;
 use std::path::{Path, PathBuf};
 // TODO(sg): Switch to serde_yaml's `apply_merge()` once it supports recursive merges, cf.
@@ -6,8 +6,8 @@ use std::path::{Path, PathBuf};
 use yaml_merge_keys::merge_keys_serde;
 
 use crate::list::{List, RemovableList, UniqueList};
-use crate::types::Mapping;
-use crate::Reclass;
+use crate::types::{Mapping, Value};
+use crate::{Reclass, SUPPORTED_YAML_EXTS};
 
 mod nodeinfo;
 
@@ -31,12 +31,11 @@ pub struct Node {
     /// Location of this node relative to `classes_path`. `None` for nodes.
     #[serde(skip)]
     own_loc: Option<PathBuf>,
+    /// Information about the node, empty (default value) for Node objects parsed from classes.
     #[serde(skip)]
     meta: NodeInfoMeta,
 }
 
-const SUPPORTED_YAML_EXTS: [&str; 2] = ["yml", "yaml"];
-
 /// Loads data from `<npath>.yml` or `<npath>.yaml`.
 fn load_file(npath: &Path) -> Result<(String, PathBuf)> {
     let mut ncontents: Result<(String, PathBuf)> =
@@ -95,11 +94,7 @@ impl Node {
         n._params = p;
 
         // Convert serde_yaml::Mapping into our own Mapping type
-        let mut params: Mapping = n._params.clone().into();
-        // Inject `_reclass_` meta parameter into parameters, return an error if someone has marked
-        // `_reclass_` or a subkey of it as constant.
-        params.insert("_reclass_".into(), n.meta.as_reclass().into())?;
-        n.parameters = params;
+        n.parameters = n._params.clone().into();
 
         Ok(n)
     }
@@ -158,11 +153,148 @@ impl Node {
 
         Ok(absclass)
     }
+
+    /// Looks up and parses `Node` from provided `class` string relative to own location.
+    ///
+    /// If the current Node's location is empty, relative class references inherently turn into
+    /// absolute references, since relative references can't escape the `r.classes_path` base
+    /// directory.
+    ///
+    /// If the class is not prefixed with one or more dots, it's looked up relative to
+    /// `r.classes_path`.
+    ///
+    /// The method extracts the the relative file path for the class in `r.classes_dir` from
+    /// `r.classes`.
+    fn read_class(&self, r: &Reclass, class: &str) -> Result<Option<Self>> {
+        let cls = self.abs_class_name(class)?;
+
+        // Lookup path for provided class in r.classes, handling ignore_class_notfound
+        let Some(cpath) = r.classes.get(&cls) else {
+            if r.ignore_class_notfound {
+                return Ok(None);
+            }
+            return Err(anyhow!("Class {cls} not found"));
+        };
+
+        // Extract the directory in which the new class is stored to use for the new class's
+        // `own_loc`.
+        let class_loc = if let Some(parent) = cpath.parent() {
+            PathBuf::from(parent)
+        } else {
+            PathBuf::new()
+        };
+
+        // Render inventory path of class based from `r.classes_path`.
+        let mut invpath = PathBuf::from(&r.classes_path);
+        invpath.push(cpath);
+
+        // Load file contents and create Node
+        let mut meta = NodeInfoMeta::default();
+        let ccontents = std::fs::read_to_string(invpath.canonicalize()?)?;
+        meta.uri = format!("yaml_fs://{}", invpath.canonicalize()?.display());
+        Ok(Some(
+            Node::from_str(meta, Some(class_loc), &ccontents)
+                .with_context(|| format!("Deserializing {cls}"))?,
+        ))
+    }
+
+    /// Merges self into other, then updates self with merged values from other
+    fn merge_into(&mut self, other: &mut Self) -> Result<()> {
+        // We use std::mem::take() here so we can merge self.applications into other.applications
+        // without having to call `clone()` twice. This doesn't destroy `self.applications` because
+        // we update `self.applications` with the result of the merge immediately afterwards.
+        let self_apps = std::mem::take(&mut self.applications);
+        other.applications.merge(self_apps);
+        self.applications = other.applications.clone();
+
+        // We use std::mem::take() here so we can merge self.classes into other.classes without
+        // having to call `clone()` twice. This doesn't destroy `self.classes` because we update
+        // `self.classes` with the result of the merge immediately afterwards.
+        let self_classes = std::mem::take(&mut self.classes);
+        other.classes.merge(self_classes);
+        self.classes = other.classes.clone();
+
+        other.parameters.merge(&self.parameters)?;
+        self.parameters = other.parameters.clone();
+        Ok(())
+    }
+
+    /// Recursively loads classes and merges loaded data into self
+    fn render_impl(&mut self, r: &Reclass, seen: &mut Vec<String>, root: &mut Node) -> Result<()> {
+        for cls in self.classes.items_iter() {
+            if seen.contains(cls) {
+                continue;
+            }
+
+            // TODO(sg): parse and render references in class names
+            let cls = cls.to_string();
+
+            let maybec = self.read_class(r, &cls);
+            let Ok(Some(mut c)) = maybec else {
+                if let Ok(None) = maybec {
+                    eprintln!("ignore missing class {cls}");
+                    continue;
+                }
+                return Err(maybec.unwrap_err());
+            };
+
+            // render class so we pick up further classes included in it
+            c.render_impl(r, seen, root)?;
+            // NOTE(sg): we don't need to merge here, since we've already mergeed into root as part
+            // of the recursive call to `render_impl()`
+
+            seen.push(cls.to_string());
+        }
+
+        // merge root into self, then update self with merged values
+        self.merge_into(root)
+    }
+
+    fn flatten_parameters(&mut self) -> Result<()> {
+        let p = std::mem::replace(&mut self.parameters, Mapping::new());
+        let f = Value::Mapping(p).flattened()?;
+        match f {
+            Value::Mapping(m) => {
+                self.parameters = m;
+                Ok(())
+            }
+            _ => Err(anyhow!(
+                "Flattened parameters are not a Mapping but instead a {}",
+                f.variant()
+            )),
+        }
+    }
+
+    /// Load included classes (recursively), and merge parameters.
+    ///
+    /// Note that this method doesn't flatten overwritten parameters.
+    pub fn render(&mut self, r: &Reclass) -> Result<()> {
+        let mut base = Node {
+            // NOTE(sg): We initialize a base node with our classes to start the class rendering
+            // process.  This roughly corresponds to Python reclass's
+            // `_get_class_mappings_entity()`.
+            classes: self.classes.clone(),
+            ..Default::default()
+        };
+        // NOTE(sg): We merge the `_reclass_` meta parameter into the base node before starting
+        // class loading. This roughly corresponds to Python reclass's
+        // `_get_automatic_parameters()`.
+        base.parameters
+            .insert("_reclass_".into(), self.meta.as_reclass().into())?;
+
+        let mut seen = vec![];
+        let mut root = Node::default();
+        base.render_impl(r, &mut seen, &mut root)?;
+        self.render_impl(r, &mut seen, &mut base)?;
+        self.flatten_parameters()
+    }
 }
 
 #[cfg(test)]
 mod node_tests {
     use super::*;
+    use crate::types::Value;
+    use std::str::FromStr;
 
     #[test]
     fn test_parse() {
@@ -170,9 +302,9 @@ mod node_tests {
             "./tests/inventory/nodes",
             "./tests/inventory/classes",
             false,
-        );
+        )
+        .unwrap();
         let n = Node::parse(&r, "n1").unwrap();
-        println!("{:#?}", n.meta);
         assert_eq!(
             n.classes,
             UniqueList::from(vec!["cls1".to_owned(), "cls2".to_owned()])
@@ -186,13 +318,6 @@ mod node_tests {
           foo: foo
         bar:
           foo: foo
-        _reclass_:
-          environment: base
-          name:
-            short: n1
-            parts: ["n1"]
-            full: n1
-            path: n1
         "#;
         let expected: serde_yaml::Mapping = serde_yaml::from_str(expected).unwrap();
         assert_eq!(n.parameters, expected.into());
@@ -379,4 +504,126 @@ mod node_tests {
         let p = c.abs_class_name(".....foo.bar").unwrap();
         assert_eq!(p, "foo.bar");
     }
+
+    #[test]
+    fn test_read_class() {
+        let r = Reclass::new(
+            "./tests/inventory/nodes",
+            "./tests/inventory/classes",
+            false,
+        )
+        .unwrap();
+        let n = Node::parse(&r, "n1").unwrap();
+        let c = n.read_class(&r, "cls1").unwrap().unwrap();
+        let expected = r#"
+        foo:
+          foo: cls1
+          bar: cls1
+          baz: cls1
+        "#;
+        let expected = Mapping::from_str(expected).unwrap();
+        assert_eq!(c.parameters, expected);
+    }
+
+    #[test]
+    fn test_read_class_relative() {
+        let r = Reclass::new(
+            "./tests/inventory/nodes",
+            "./tests/inventory/classes",
+            false,
+        )
+        .unwrap();
+        let n = Node::parse(&r, "n1").unwrap();
+        let c1 = n.read_class(&r, "nested.cls1").unwrap().unwrap();
+        let c2 = c1.read_class(&r, ".cls2").unwrap().unwrap();
+        let expected = r#"
+        foo:
+          foo: nested.cls2
+        "#;
+        let expected = Mapping::from_str(expected).unwrap();
+        assert_eq!(c2.parameters, expected);
+    }
+
+    #[test]
+    fn test_render_n1() {
+        let r = Reclass::new(
+            "./tests/inventory/nodes",
+            "./tests/inventory/classes",
+            false,
+        )
+        .unwrap();
+        let mut n = Node::parse(&r, "n1").unwrap();
+        assert_eq!(
+            n.classes,
+            UniqueList::from(vec!["cls1".to_owned(), "cls2".to_owned()])
+        );
+        assert_eq!(
+            n.applications,
+            RemovableList::from(vec!["app1".to_owned(), "app2".to_owned()])
+        );
+
+        n.render(&r).unwrap();
+
+        let expected = r#"
+        foo:
+          foo: foo
+          bar: cls2
+          baz: cls1
+        bar:
+          foo: foo
+        _reclass_:
+          environment: base
+          name:
+            short: n1
+            parts: ["n1"]
+            full: n1
+            path: n1
+        "#;
+        let mut expected: Value = Mapping::from_str(expected).unwrap().into();
+        expected.flatten().unwrap();
+        let params: Value = n.parameters.into();
+
+        assert_eq!(params, expected);
+    }
+
+    #[test]
+    fn test_render_n2() {
+        let r = Reclass::new(
+            "./tests/inventory/nodes",
+            "./tests/inventory/classes",
+            false,
+        )
+        .unwrap();
+        let mut n = Node::parse(&r, "n2").unwrap();
+        assert_eq!(n.classes, UniqueList::from(vec!["nested.cls1".to_owned()]));
+        assert_eq!(n.applications, RemovableList::from(vec![]));
+
+        n.render(&r).unwrap();
+
+        assert_eq!(
+            n.classes,
+            UniqueList::from(vec!["nested.cls2".to_owned(), "nested.cls1".to_owned()])
+        );
+
+        let expected = r#"
+        foo:
+          foo: nested.cls1
+          bar: n2
+        bar: bar
+        _reclass_:
+          environment: base
+          name:
+            short: n2
+            parts: ["n2"]
+            full: n2
+            path: n2
+        "#;
+        let mut expected: Value = Mapping::from_str(expected).unwrap().into();
+        expected.flatten().unwrap();
+        let params: Value = n.parameters.into();
+
+        dbg!(&params);
+
+        assert_eq!(params, expected);
+    }
 }


Test patch:
diff --git a/tests/broken-inventory/classes/foo.bar.yml b/tests/broken-inventory/classes/foo.bar.yml
new file mode 100644
index 0000000..e69de29
diff --git a/tests/broken-inventory/classes/foo/bar.yml b/tests/broken-inventory/classes/foo/bar.yml
new file mode 100644
index 0000000..e69de29
diff --git a/tests/inventory/classes/cls1.yml b/tests/inventory/classes/cls1.yml
index e0c9eb5..0f7f7c2 100644
--- a/tests/inventory/classes/cls1.yml
+++ b/tests/inventory/classes/cls1.yml
@@ -1,1 +1,5 @@
-parameters: {}
+parameters:
+  foo:
+    foo: cls1
+    bar: cls1
+    baz: cls1
diff --git a/tests/inventory/classes/cls2.yml b/tests/inventory/classes/cls2.yml
index e0c9eb5..57ccffb 100644
--- a/tests/inventory/classes/cls2.yml
+++ b/tests/inventory/classes/cls2.yml
@@ -1,1 +1,4 @@
-parameters: {}
+parameters:
+  foo:
+    foo: cls2
+    bar: cls2
diff --git a/tests/inventory/classes/nested/cls1.yml b/tests/inventory/classes/nested/cls1.yml
new file mode 100644
index 0000000..2f048d2
--- /dev/null
+++ b/tests/inventory/classes/nested/cls1.yml
@@ -0,0 +1,5 @@
+classes:
+  - .cls2
+parameters:
+  foo:
+    foo: nested.cls1
diff --git a/tests/inventory/classes/nested/cls2.yml b/tests/inventory/classes/nested/cls2.yml
new file mode 100644
index 0000000..19c7617
--- /dev/null
+++ b/tests/inventory/classes/nested/cls2.yml
@@ -0,0 +1,3 @@
+parameters:
+  foo:
+    foo: nested.cls2
diff --git a/tests/inventory/nodes/n2.yml b/tests/inventory/nodes/n2.yml
new file mode 100644
index 0000000..23171d1
--- /dev/null
+++ b/tests/inventory/nodes/n2.yml
@@ -0,0 +1,6 @@
+classes:
+  - nested.cls1
+parameters:
+  foo:
+    bar: n2
+  bar: bar
diff --git a/tests/test_import.py b/tests/test_import.py
index 180f666..90f010e 100644
--- a/tests/test_import.py
+++ b/tests/test_import.py
@@ -1,17 +1,20 @@
 import reclass_rs
+import pytest
+import os
 
 
 def test_import_new():
-    r = reclass_rs.Reclass()
+    r = reclass_rs.Reclass(
+        "./tests/inventory/targets", "./tests/inventory/classes", True
+    )
     assert r is not None
-    assert r.nodes_path == "./inventory/nodes"
-    assert r.classes_path == "./inventory/classes"
-    assert not r.ignore_class_notfound
+    assert r.nodes_path == "./tests/inventory/targets"
+    assert r.classes_path == "./tests/inventory/classes"
+    assert r.ignore_class_notfound
 
 
-def test_import_new_args():
-    r = reclass_rs.Reclass("./test/targets", "./test/classes", True)
-    assert r is not None
-    assert r.nodes_path == "./test/targets"
-    assert r.classes_path == "./test/classes"
-    assert r.ignore_class_notfound
+def test_import_raises():
+    with pytest.raises(ValueError) as exc:
+        r = reclass_rs.Reclass("./foo", "./bar")
+
+    assert "Error while discovering classes" in str(exc.value)
diff --git a/tests/test_nodeinfo.py b/tests/test_nodeinfo.py
index 09cef83..39c2bae 100644
--- a/tests/test_nodeinfo.py
+++ b/tests/test_nodeinfo.py
@@ -18,6 +18,28 @@ def test_nodeinfo_n1():
                 "short": "n1",
             },
         },
-        "foo": {"foo": "foo"},
+        "foo": {"foo": "foo", "bar": "cls2", "baz": "cls1"},
         "bar": {"foo": "foo"},
     }
+
+
+def test_nodeinfo_n2():
+    r = reclass_rs.Reclass(
+        nodes_path="./tests/inventory/nodes", classes_path="./tests/inventory/classes"
+    )
+    n = r.nodeinfo("n2")
+    assert n.applications == []
+    assert n.classes == ["nested.cls2", "nested.cls1"]
+    assert n.parameters == {
+        "_reclass_": {
+            "environment": "base",
+            "name": {
+                "full": "n2",
+                "parts": ["n2"],
+                "path": "n2",
+                "short": "n2",
+            },
+        },
+        "foo": {"foo": "nested.cls1", "bar": "n2"},
+        "bar": "bar",
+    }
diff --git a/tests/test_py.rs b/tests/test_py.rs
index 34834a6..2f52cb9 100644
--- a/tests/test_py.rs
+++ b/tests/test_py.rs
@@ -7,7 +7,13 @@ use reclass_rs::Reclass;
 fn test_reclass() {
     pyo3::prepare_freethreaded_python();
     Python::with_gil(|py| {
-        let r = Reclass::default().into_py(py);
+        let r = Reclass::new(
+            "./tests/inventory/nodes",
+            "./tests/inventory/classes",
+            false,
+        )
+        .unwrap()
+        .into_py(py);
         let locals = PyDict::new(py);
         locals.set_item("r", r).unwrap();
         py.run(