- 
                Notifications
    
You must be signed in to change notification settings  - Fork 2
 
Improve selftest of JsonPreprocessor package #30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| 
           Hi Son, I wanted the python-jsonpreprocessor repository to support the TestTrigger mechanism. Therefore I added a TestExecutor 'executerobottest.py' to the atest folder. The execution causes the following error message: This happens in all tests. Therefore all tests fail. The atest folder already contains a script 'run.py' that also executes the test. With 'run.py' this problem does not occur. Most of the tests are passed, except "Test Sub Data Structure 06": Test Sub Data Structure 06 :: Updated more than 1 parameter with n... | FAIL | The main difference between executerobottest.py and run.py is: run.py uses robot.run() to execute the tests, whereas executerobottest.py uses subprocess.call(). And executerobottest.py does some things more, like the handling the algebraic sign of the return value and passing command lines to the Robot Framework. We have now two possibilities: 
 I would prefer the second option because this would make it possible to handle the test execution in every repository in the same way with the same mechanisms, like described in It would also be interesting to learn about the root cause for this deviating behavior.  | 
    
| 
           Hello Holger, I already deleted 'run.py' and replaced by 'run.bat' for Windows and 'run.sh' for linux.  | 
    
| 
           Hi Son, for executing pytests you have to use the executepytest.py I have updated  | 
    
| 
           Hi both,  | 
    
| "${gPreproString}": "Override string param in imported file.", | ||
| "params":{ | ||
| "global":{ | ||
| "glob":{ | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi  Son,
where is "glob" coming from?
I was not asked for this and I did therefore not agree to this change.
I'm sorry that you need to do some rework now.
Python has a "global" keyword for global parameters:
https://www.w3schools.com/python/python_variables_global.asp
We use what users expect. Therefore "global" is the correct key here to indicate a global scope.
In addition "glob" has another meaning in english language.
Thank you,
Thomas
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello Thomas,
Due to the "global" is reserved keyword of Python, so some scenarios of pytest failed when I name the key of Json object is "global".
That the reason I just changed this key name of Json configuration file.
Thank you,
Son
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Son,
O.K. I understand your motivation. But this is no solution to make tests running.
Can not be that the test tool makes a global syntax change required.,,
Can you please discuss with Cuong if he has an idea what to do.
Thank you,
Thomas
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Son,
what is the status here?
Thank you,
Thomas
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello Thomas,
I created new commit  to use object name 'global' instead of 'glob'.
I also run this selftest on both Windows and OSD6.
Thank you,
Son
| 
           Hi Son,  | 
    
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Son,
please check my findings.
Thank you,
Thomas
| "${gPreproString}": "Override string param in imported file.", | ||
| "params":{ | ||
| "global":{ | ||
| "glob":{ | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Son,
what is the status here?
Thank you,
Thomas
| 
           Hi Son,  | 
    
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Son,
looks good to me except of the use of "glob" instead of "global".
What is the status here?
Thank you,
Thomas
          
 Hello Thomas, I created new commit  to use object name 'global' instead of 'glob'. Thank you,  | 
    
…rework of the atest)
| 
           Hi Son, How did you solve the "glob" -> "global" problem now. I didn't find the place in the changed code. Thank you,  | 
    
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Son,
looks good to me now!
Thank you,
Thomas
Update selftest using pytest for JsonPreprocessor package