Added tests for format module #7
Conversation
I have added initial tests for the format module verifying outputs, it appeared that the outputs that were passed as examples to the pipeline differed from those generated. It seemed some column headers were mangled and there may have been differences in fields generated. I have updated the test data to reflect the more recent version of locidex
I have added initial tests for the format module verifying outputs, it appeared that the outputs that were passed as examples to the pipeline differed from those generated. It seemed some column headers were mangled and there may have been differences in fields generated. I have updated the test data to reflect the more recent version of locidex
| "input": "/Users/jrobertson/PycharmProjects/locidex/locidex/example/format_db_mlst_in", | ||
| "outdir": "/Users/jrobertson/PycharmProjects/locidex/locidex/example/format_db_mlst_out", | ||
| "input": "locidex/example/format_db_mlst_in", | ||
| "outdir": "/tmp/pytest-of-mwells/pytest-115/build0", |
There was a problem hiding this comment.
I was wondering if you can use a global variables for paths instead of absolute paths for better portability such as TEST_ROOT = os.path.dirname(__file__). Take a look at https://github.com/phac-nml/mob-suite/blob/master/mob_suite/tests/test_mobtyper_vs_biomarker_report.py
| from dataclasses import dataclass | ||
|
|
||
| EXPECTED_DATA_OUT = "locidex/example/format_db_mlst_out" | ||
| TEST_DATA = "locidex/example/format_db_mlst_in" |
There was a problem hiding this comment.
You can also even make it more general via __file__ such as PACKAGE_ROOT = os.path.join(os.path.dirname(mob_suite.__file__),/example/...
| def test_format(cmd_args): | ||
| format.run(cmd_args) | ||
|
|
||
| def test_outputs(output_directory): |
There was a problem hiding this comment.
Not sure what is the purpose of this function? I suspect it is to make sure the listed files are in the same order. Also on Mac or other OS there might be hidden files such as .DS_Store that could break this assertion
| "force" | ||
| ] | ||
| result_json = "results.json" | ||
| expected = os.path.join(EXPECTED_DATA_OUT, result_json) |
There was a problem hiding this comment.
I think the actual vs expected absolute paths need an assertion statement here so the error is more cleaner if they happen do differ
| expected = os.path.join(EXPECTED_DATA_OUT, result_json) | ||
| actual = os.path.join(output_directory, result_json) | ||
| with open(actual, 'r', encoding='utf8') as act, open(expected, 'r', encoding='utf8') as expc: | ||
| act_js = json.load(act)[primary_key] |
There was a problem hiding this comment.
I would better first test if the key is found in the JSON loaded object and display more cleaner error message instead of key not found error. Also it would not be clear if tis not found in actual vs expected paths.
if key_to_check in json.load(act).keys():
print(f"Key '{key_to_check}' exists.")
else:
print(f"Key '{key_to_check}' does not exist.")
I have added initial tests for the format module verifying outputs, it appeared that the outputs that were passed as examples to the pipeline differed from those generated. It seemed some column headers were mangled and there may have been differences in fields generated. I have updated the test data to reflect the more recent version of locidex