-
Notifications
You must be signed in to change notification settings - Fork 42
Py3 support + changes for new RBFW and SeleniumLibrary by aaltat #231
base: master
Are you sure you want to change the base?
Conversation
Strange. The unittest on my windows machine has passed. I'll go checking with OSX again |
Well, it does look pretty good. There is only one failure in the Python 3.6. @andriyko should we give him the push rights or what we should do? |
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.
@idxn , could please update the PR description giving short answers to these questions: what? why? how? For example:
What?
Move to Python3. Drop support of Python2.
Why?
Because nobody uses Python2 anymore )
How?
- change1
- change2
- etc
Thank you for your contribution!
I had one comment. In some point it would be vice to fix the tests. too |
Sure, i'll try to get it fixed |
add missing new line at the end of the file
@aaltat @andriyko I have taken a look and find my suspect. In testdata, resource file importing in 'test\resource\test_data\real_suite\resource\resource1\real_suite_resource.robot' does not exist in the project as follow:
If it's intended to, the reason it fails on unix based system is that the order of the queue list is not the same as windows. In windows, it's like this:
On the other hand, in unix based system, it's like this:
Currently, our implementation will raise an exception if it cannot find the file so it won't parse any files in the latter. It causes our unit test to fail. If we intends to use the test data like this for specific reason, I'll need to make the changes to continue parse the file in the queue so the test_scanner.TestScanner.test_queue_populated unittest will pass and it quite makes sense for me that the scanner should not stop a scanning process because of non-existing file. |
… the version and correct the typo
Please ignore the first comment. It misunderstood something :( I'll take a look again. |
I am having a flu and laying in the bed. I will take a look later. |
Take some rest. I'm not in rush though. I'll try to get it resolve on my own. |
@@ -3,10 +3,10 @@ | |||
|
|||
try: | |||
from parser_utils.file_formatter import lib_table_name | |||
from noralize_cell import get_data_from_json | |||
from normalize_cell import get_data_from_json | |||
except ImportError: |
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.
Perhaps not in this commit, but if you are going to support only Python 3, then these type of try/except are not needed.
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.
Ok. I think I would get this merged first and do the refactoring again in the coming pull request.
Well, I am pretty happy with the changes. Although looking in to why those test fails would be good to go . |
Move to Python3. Drop support of python2 because python2 will be end of life in the year of 2019 so it would be better to move forward.