search

test-fullautomation / python-jsonpreprocessor

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

Namsonx/task/stabi branch #238

Closed namsonx closed 2 months ago

namsonx commented 2 months ago

Hello Thomas, Hello Holger,

I create this pull-request to remove globals scope out of all exec() executions as you mentioned before to avoid some risks may be appear in future. I defined self.JPGlobals = {} in the __init__ of CJsonPreprocessor to manage root parameters in json files.

Thank you, Son

namsonx commented 2 months ago

Hi Son,

I do not understand this. In test-fullautomation/robotframework-testsuitesmanagement#251 I asked a question. The question is not answered. The behavior of the JsonPreprocessor w.r.t. 251 is the same like before.

What is the status now?

Hello Holger,

Sorry for the confusion, this is my mistake, this pull-request is not related to your question in TSM ticket 251. Please ignore my comment about about TSM ticket 251.

Thank you, Son

test-fullautomation commented 2 months ago

Hi @namsonx , if you provide a release_info file (from enduser perspective. e.g. this and this is possible now - with small code example - I would merge. Than you, Thomas

HolQue commented 2 months ago

Hi Son,

the wording needs to be adapted:

Not: Reason: Empty or special character is detected in pair of square brackets. But: Reason: A pair of square brackets is empty or contains not allowed characters.

Not: Slicing is currently not supported! But: Slicing is not supported!

(no need to mention 'currently', because then customers will assume that slicing might be available in near future already)

Not: 'Invalid expression while handling the parameter '${listP.${keyP}}'.' But: 'Invalid expression found: '${listP.${keyP}}'.'

But nevertheless: This expression itself is not really invalid. It's a data type issue: List indices are expected to be positive integers, and not strings. Later this should be reworked.

02.04.2024 Retest successful.

namsonx commented 2 months ago

Hi Son,

the wording needs to be adapted:

Not: Reason: Empty or special character is detected in pair of square brackets. But: Reason: A pair of square brackets is empty or contains not allowed characters.

Not: Slicing is currently not supported! But: Slicing is not supported!

(no need to mention 'currently', because then customers will assume that slicing might be available in near future already)

Not: 'Invalid expression while handling the parameter '${listP.${keyP}}'.' But: 'Invalid expression found: '${listP.${keyP}}'.'

But nevertheless: This expression itself is not really invalid. It's a data type issue: List indices are expected to be positive integers, and not strings. Later this should be reworked.

Hello Holger,

Thank you for your review, I updated error messages as your suggestion

HolQue commented 2 months ago

Hi Son,

a tiny wish only:

Please remove the part "Please update the expression" from following error message:

'Slicing is not supported! Please update the expression '${testlist}[1:]'.'

In case of an error, it is obvious that the user has to do something to avoid this error. There is no need to mention this explicitly. Otherwise you would have to add such a statement to every error message.

Let's keep it short:

'Slicing is not supported (expression: '${testlist}[1:]').'