test-fullautomation / python-jsonpreprocessor

A preprocessor for json files
Apache License 2.0
2 stars 2 forks source link

Cyclic imported JSON file #78

Closed HolQue closed 8 months ago

HolQue commented 12 months ago

The error message for cyclic imported JSON files is very hard to capture:

json file 'D:\ROBFW\Dev\JsonPreprocessorTest\config_tsm\tsm-test_config_cyclic.1.jsonp': 'json file 'D:\ROBFW\Dev\JsonPreprocessorTest\config_tsm\import.cyclic\tsm-test_config_cyclic.2.jsonp': 'json file 'D:\ROBFW\Dev\JsonPreprocessorTest\config_tsm\import.cyclic\import.cyclic\tsm-test_config_cyclic.3.jsonp': 'Cyclic imported json file 'D:\ROBFW\Dev\JsonPreprocessorTest\config_tsm\tsm-test_config_cyclic.1.jsonp'!'''

Because it's only one line, without any line breaks.

In the past we decided to wrap every path in single quotes - for better readability. But in case of a cyclic import this error message seems to be a nested one. It is therefore unfavorable to use quotes here around the file paths, because the quotes are also nested. See '!''' at end of error message. This makes no sense.

In CJsonPreprocessor.py I found the following line of code, that causes the error message:

raise Exception(f"json file '{jFile}': '{error}'")

I gave it a try and modified the line to:

raise Exception(f"JSON file: {jFile}\n{error}")

This is the outcome now:

JSON file: D:\ROBFW\Dev\JsonPreprocessorTest\config_tsm\tsm-test_config_cyclic.1.jsonp
JSON file: D:\ROBFW\Dev\JsonPreprocessorTest\config_tsm\import.cyclic\tsm-test_config_cyclic.2.jsonp
JSON file: D:\ROBFW\Dev\JsonPreprocessorTest\config_tsm\import.cyclic\import.cyclic\tsm-test_config_cyclic.3.jsonp
Cyclic imported json file 'D:\ROBFW\Dev\JsonPreprocessorTest\config_tsm\tsm-test_config_cyclic.1.jsonp'!

Much better now in my opinion.

Please give this change a try.

General hint: The abbreviation JSON should be written in capital letters.

test-fullautomation commented 11 months ago

Hi Son, please look into this. Expectation is what Holger writes. Thank you, Thomas

test-fullautomation commented 8 months ago

Hi @namsonx , what is the status of this ticket? Thank you, Thomas

HolQue commented 8 months ago

Error message is improved. Ticket can be closed.

HolQue commented 8 months ago

My mistake. I tested in the context of the Testsuites Management. There the error message is well formatted.

But in between I added the same test case to the self test of the JsonPreprocessor. And the error message looks the same as described above. Therefore the issue is still not solved.

Like I mentioned above: Please change the line

raise Exception(f"json file '{jFile}': '{error}'")

to:

raise Exception(f"JSON file: {jFile}\n{error}")

Not a big thing; can be done quickly.

namsonx commented 8 months ago

Hello Holger,

I created the commit ec3a6255 to update error message log according to your suggestion.

Thank you, Son

HolQue commented 8 months ago

Solved with

https://github.com/test-fullautomation/python-jsonpreprocessor/pull/151

Ticket can be closed again.