Skip to content
This repository was archived by the owner on Nov 2, 2020. It is now read-only.

Py3 support + changes for new RBFW and SeleniumLibrary by aaltat#231

Open
idxn wants to merge 22 commits into
andriyko:masterfrom
idxn:py3
Open

Py3 support + changes for new RBFW and SeleniumLibrary by aaltat#231
idxn wants to merge 22 commits into
andriyko:masterfrom
idxn:py3

Conversation

@idxn

@idxn idxn commented Feb 16, 2019

Copy link
Copy Markdown

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.

  • convert to py3 using 2to3 script
  • use the latest RBFW and SeleniumLibrary for unit/acceptance test
  • Fix some of the typos

@idxn

idxn commented Feb 16, 2019

Copy link
Copy Markdown
Author

Strange. The unittest on my windows machine has passed. I'll go checking with OSX again

Comment thread dataparser/data_parser/data_parser.py Outdated
@aaltat

aaltat commented Feb 17, 2019

Copy link
Copy Markdown
Collaborator

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?

@andriyko andriyko left a comment

Copy link
Copy Markdown
Owner

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!

Comment thread Robot.sublime-settings Outdated
Comment thread command_helper/get_keyword.py Outdated
Comment thread command_helper/workspace_objects.py Outdated
Comment thread commands/query_completions.py Outdated
Comment thread dataparser/data_queue/scanner.py
Comment thread dataparser/parser_utils/util.py Outdated
@aaltat

aaltat commented Feb 21, 2019

Copy link
Copy Markdown
Collaborator

I had one comment. In some point it would be vice to fix the tests. too

@idxn

idxn commented Feb 22, 2019

Copy link
Copy Markdown
Author

Sure, i'll try to get it fixed

idxn and others added 2 commits February 22, 2019 15:20
@idxn

idxn commented Feb 25, 2019

Copy link
Copy Markdown
Author

@andriyko @aaltat Do I need to get all the unittest fixed or just the one in py3.6 to get this pull merged?

@idxn

idxn commented Feb 25, 2019

Copy link
Copy Markdown
Author

@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:

Resource ../../resource2/real_suite_resource.robot

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:

[SeleniumLibrary, xxx, ../../resource2/real_suite_resource.robot]

On the other hand, in unix based system, it's like this:

[ ../../resource2/real_suite_resource.robot, xxx, SeleniumLibrary]

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.

@idxn

idxn commented Feb 26, 2019

Copy link
Copy Markdown
Author

Please ignore the first comment. It misunderstood something :( I'll take a look again.

@aaltat

aaltat commented Feb 26, 2019

Copy link
Copy Markdown
Collaborator

I am having a flu and laying in the bed. I will take a look later.

@idxn

idxn commented Feb 26, 2019

Copy link
Copy Markdown
Author

Take some rest. I'm not in rush though. I'll try to get it resolve on my own.

Comment thread command_helper/get_keyword.py Outdated
Comment thread command_helper/get_keyword.py
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:

Copy link
Copy Markdown
Collaborator

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.

@idxn idxn Mar 3, 2019

Copy link
Copy Markdown
Author

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.

@aaltat

aaltat commented Mar 1, 2019

Copy link
Copy Markdown
Collaborator

Well, I am pretty happy with the changes. Although looking in to why those test fails would be good to go .

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants