kellyjonbrazil / jc

CLI tool and python library that converts the output of popular command-line tools, file-types, and common strings to JSON, YAML, or Dictionaries. This allows piping of output to tools like jq and simplifying automation scripts.
MIT License
7.91k stars 210 forks source link

Feature: generic fixtures tests #529

Closed muescha closed 9 months ago

muescha commented 9 months ago

I notice numerous tests using fixtures in the following format:

assertEqual(jc.parser.myparser.parse(command_output), expected_json)

To enhance code organization and eliminate redundancy, I've conducted a general search for the *.out and *.json combination.

Perhaps it would be beneficial to extract these functions into a testutils.py file. This way, we can streamline and remove repetitive code.

muescha commented 9 months ago

it can also optimised not to preload all the fixture files into class variables before running the tests

muescha commented 9 months ago

Note: it is just a draft to see if the idea is welcome, then it can be refactored to fit better the needs

kellyjonbrazil commented 9 months ago

There are a lot of ways to automate the tests, but I have actually made a conscious effort to not be fancy and be more manual with the tests. This is a style choice but for me it gives me clearer error messages since the broken test and its fixture file are explicitly defined. The preloading of the fixtures seems to help improve the test speed (1400 tests in about 8 seconds on my laptop).

I would say parser authors are free to write tests however they are comfortable and if there are some helper functions we can put in a module, then I'm fine with that.

I find that being explicit and manual with the tests helps me to really understand what's going on with a parser and sometimes makes me think of some edge cases that need to be addressed.

muescha commented 9 months ago

if there are some helper functions we can put in a module, then I'm fine with that.

yes it should be an opt-in by the author, and additional the automatic fixtures are marked with double dash --

it gives me clearer error messages since the broken test and its fixture file are explicitly defined.

for this is wrapped in subTest with the info about the fixtures file name

The preloading of the fixtures seems to help improve the test speed (1400 tests in about 8 seconds on my laptop).

thats interesting - you mean when you reuse a fixture?

muescha commented 9 months ago

extracted to utils file ec32934

now test files with no special checks are short as:

kellyjonbrazil commented 9 months ago

Very nice! We'll just need to add good docstrings to document how/why to use each function.

We can also add a test template file called tests/templates/_test_foo_simple.py that shows how to create a simple test file with some notes on how to properly place and name the fixture files.

muescha commented 9 months ago

I discovered a generic function for obtaining the caller's parser_name, but the stack differs when calling it from the test_x.py file compared to the utils_for_test.py file.

While it's conceivable to navigate through the stack until the first file with the name test_*, I believe this approach introduces excessive magic. Would it be more advisable to eliminate this method?

https://github.com/muescha/jc/blob/feature/generic-tests/tests/utils_for_test.py#L19-L25

muescha commented 9 months ago

added template and some documentation. I am open for a rewording suggestions :)

kellyjonbrazil commented 9 months ago

Looks good - let me know if you are ready for me to merge. I'll make a couple documentation updates on my side.

muescha commented 9 months ago

i am ready to merge