test-fullautomation / python-jsonpreprocessor

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

Side effects of latest changes (2) #276

Open HolQue opened 1 month ago

HolQue commented 1 month ago

Latest changes in JsonPreprocessor have side effects.

(1)

"newparam" : ${indexP.${indexP}}

An integer cannot be applied to an integer.

Previous error message:

'Invalid expression found: '${indexP.${indexP}}'.'

New error message:

'The variable '${indexP}[0]' is not available!'

This is a deterioration. The previous error message was fine.

In pure Python

index = 2
param = index[index]

This would cause a

TypeError: 'int' object is not subscriptable

This would also be OK.

But telling that a variable does not exist, is misleading. There is no variable missing. This is a type issue. In case you rework something, you already can consider: https://github.com/test-fullautomation/python-jsonpreprocessor/issues/257#issuecomment-2037322355 It has already been decided not to use the term "variable" any more. Use "parameter" instead.

(2)

Same with a string applied to an integer

"newparam" : ${indexP.${keyP}}

Previous error message:

'Invalid expression found: '${indexP.${keyP}}'.'

New error message:

'The variable '${indexP}['A']' is not available!'

Also in this case Python would throw:

TypeError: 'int' object is not subscriptable

Therefore same issue like in (1):

Telling that a variable does not exist, is misleading. There is no variable missing. This is a type issue. In case you rework something, you already can consider: https://github.com/test-fullautomation/python-jsonpreprocessor/issues/257#issuecomment-2037322355 It has already been decided not to use the term "variable" any more. Use "parameter" instead.

(3)

Also a dictionary cannot be applied to an integer.

"newparam" : ${indexP.${dictP}}

Previous error message:

'Invalid expression found: '${indexP.${dictP}}'.'

New error message:

''NoneType' object is not subscriptable'

Same issue like in (1) and (2).

This error message is not helpful. Please restore the previous one or take over the error message from Python (TypeError: 'int' object is not subscriptable).

Same with

"newparam" : ${indexP.${listP}}

(4)

A string cannot be applied to a string.

"newparam" : ${keyP.${keyP}}

Previous error message:

'Invalid expression found: '${keyP.${keyP}}'.'

New error message:

'The variable '${keyP}['A']' is not available!'

Same issue like before: There is no variable missing. This is a type issue. This expression ${keyP.${keyP}} is simply an invalid combination of data types.

In such a case Python would throw:

TypeError: string indices must be integers

Please take over the error message from Python.

In general:

I already mentioned this several times: In case of expressions like ${X}[Y] you urgently have to consider the data type of X and Y.

When you implement changes you also should check impacts and side effects of your changes. This would avoid the need to spend time on writing and tracking issues like this.

You should try to get a better overview about your code and align the implementation. Often things work well, but not everywhere in the same way.

For example:

"${keyP}['${indexP}']" : "newvalue"

This error (string by mistake used as index) is already detected properly:

'${keyP} expects integer as index. Got string instead in ${keyP}['${indexP}']'

This corresponds to (4):

"newparam" : ${keyP.${keyP}}

Therefore I expect the same error message.

namsonx commented 3 weeks ago

Hello Holger,

This ticket was implemented on stabi branch.

Thank you, Son

HolQue commented 1 week ago

(1), (2) and (4) solved. (3) still open.

namsonx commented 1 week ago

Hello Holger,

I had updated error message log for case (3), the error message of this case looks like below:

Exception: Could not resolved the expression ${indexP.${dictP}}. The datatype of ${dictP} must be 'str' or 'int' not 'dict'.

Thank you, Son

HolQue commented 1 week ago

Hi Son,

what are you doing? This error message is wrong! An 'int' object is not subscriptable. In (3) I already wrote what is expected.

namsonx commented 1 week ago

Hello Holger,

In case (3) "newparam" : ${indexP.${dictP}}, there are 2 causes of error I can observe:

Anyway, I had implemented the first option as you expected and push the commit to the stabi branch. Now, the error message is Exception: Could not resolved the expression '${indexP.${dictP}}'. Reason: 'int' object is not subscriptable

Thank you,

HolQue commented 1 week ago

Hi Son,

very good that you enabled the first option. Because this is more plausible for users than the second one.

And please adapt the wording. Either:

could not resolve expression ...

or:

expression ... could not be resolved

namsonx commented 1 week ago

Hello Holger,

I had updated the wording as your suggestion.

Thank you, Son

HolQue commented 1 week ago

Retest successful. Issue can be closed.