search

network-unit-testing-system / nuts

NUTS defines a desired network state and checks it against a real network using pytest and nornir.
https://nuts.readthedocs.io/
MIT License
106 stars 5 forks source link

Decide on internal structure of nuts (aka: The Dinosaur) #19

Open bytinbit opened 3 years ago

bytinbit commented 3 years ago

Nuts uses a lot of pytest internals (namely pytest's Python API) to generate tests (yamlloader.py: NutsTestFile). This works, but abuses a lot of internal functionalities of pytest and makes the whole project rest on unstable foundations.

Problem with NutsTestFile: Collects YAML-tests, and loads Python-Code in addition. It should be either loading tests from YAML-Files or loading tests from Python-Code. It is possible to collect YAML-tests and load Python-Code, but the current implementation is a pytest-plugin that should implement the loading and collection itself, but still uses pytest's interna to do so.

The decision on how to proceed with nut's structure leads to two different paths:

Variation 1:

Keep the structure as is.

Pro: Use pytests's functionalities. Use fixtures in base tests and custom markers (@pytest.mark.nuts("host,network_instance,interfaces")).

Con: Uses pytest's internals. If something changes within pytest, it might lead to nuts' not function properly anymore.

Variation 2:

Change the structure: Write code to load yaml and generate tests (implement NutsTestFel without deriving from pytest.Module).

Pro: Pytest delegated to being a runner. Pytest internal not abused for the plugin's functionality.

Con: Not possible to use pytest's fixtures and markers, i.e. not possible to use fixtures and custom markers in base test. ==> Can be a chance, gives more control to developers and makes code more understandable. Nevertheless: Requires big refactoring (actually: complete refactoring) to current code base with lots of conceptualization beforehand.

Variation 3:

Python-Files are the master, not YAML. Python-File is collected from pytest, as plugin we simply add the YAML-File. Maybe not possible, but should be evaulated.

bytinbit commented 2 years ago

We tried to implement a solution in this branch but did not succeed, since we were not aware of all the implications of the change. Out of the code changes in that branch however this very issue here was created.

bytinbit commented 2 years ago

Variation 3 is explored in a branch referring to #41