andriyko / sublime-robot-framework-assistant

Robot Framework plugin for Sublime Text3
MIT License
109 stars 43 forks source link

Solution for issue #209 #210

Closed oeick closed 6 years ago

oeick commented 6 years ago

Added a method to remove GIVEN/WHEN/THEN-prefix for JumpToKeyword, so that it is now possible to jump to the keyword definition.

See issue #209

aaltat commented 6 years ago

The Travis build did fail, I did not look why, but I did restart the build. If it fails second time, could you look where the failure happens.

Noordsestern commented 6 years ago

We cannot reproduce the error.

Maybe the dependency to selenium library is the problem. The unit test tries to join a path (test_get_keyword_from_library.py, line 175): return path.join(self.s2l, 'keywords', '_element.py')

We are using an old version of selenium library (1.8.0) where an keywords/_elements.py exists. However, the newest selenium library does only contain source files without leading underscore.

Does Travis load the correct selenium library suitable for that unit test?

Noordsestern commented 6 years ago

Yes, I believe, selenium2library is the problem. Nowadays, Travis installs version 3.0.0, while it used to be version 1.8.0 (compare to pending pull request #202 that passed CI)

aaltat commented 6 years ago

This is true, the tests are planned for Selenium2Library 1.8.0 but Travis configuration installs the latest, which is 3.0.0. And this causes the test on this PR to fail. I will fix it.

I looked at your solution and it's quite clever and simple, but there are few things to discuss.

1) Should the prefix removal be configurable? 2) Should the prefixes be also configurable? Robot Framework supports Given, When, Then, And and But as prefixes [1] I can imagine many cases that where I don't want to remove all the prefixes from the keywords. It is possible to put the prefixes in the configuration file and create the regexp based on the configuration.

[1] http://robotframework.org/robotframework/latest/RobotFrameworkUserGuide.html#behavior-driven-style

oeick commented 6 years ago

Very good points. I like the idea of a configurable prefix-filter and will try to enhance the patch.

aaltat commented 6 years ago

I was thinking this little bit and making it configurable what prefixes are stripped should be enough. It would be best to have a new parameter, list type, in here: https://github.com/andriyko/sublime-robot-framework-assistant/blob/master/Robot.sublime-settings

The settings are read by this object: https://github.com/andriyko/sublime-robot-framework-assistant/blob/master/setting/setting.py

Which is used to read either the project specific settings or return the settings from the Sublime settings.

aaltat commented 6 years ago

I did fix the problem caused by the new Selenium2Library version. The fix in the master branch and next time you update the PR, could you get the changes from master so that Travis will show correct results.

aaltat commented 6 years ago

Looks good from a mobile screen. I need to double check from a bigger screen, because sometimes I miss things from a smaller screen.