New keyword Get Browser Capabilities. #981
Conversation
HelioGuilherme66 force-pushed the get_browser_capabilities branch 2 times, most recently from 3b7b481 to 24baf59 Compare 
aaltat left a comment
aaltat left a comment There was a problem hiding this comment.
The code side looks OK but there are few things which needs to be fixed before this can be merged.
| % self.browser.session_id) | ||
| | ||
| @keyword | ||
| def get_browser_capabilities(self): |
There was a problem hiding this comment.
Write: Gets current browser capabilities as dictionary
| def get_browser_capabilities(self): | ||
| """Gets current browser capabilities from webdriver. | ||
| Returns a Dictionary with webdriver (browser) desired capabilities attributes. |
There was a problem hiding this comment.
Instead of explaining how RF dictionary works, it might be better idea to link in RF user guide: http://robotframework.org/robotframework/latest/RobotFrameworkUserGuide.html#dictionary-variables and to the chapter which explains DotDic usage.
There was a problem hiding this comment.
I am keeping as it is. Better explicit than implicit. We don't have much examples on how to use SeleniumLibrary with dictionary attributes.
| Returns a Dictionary with webdriver (browser) desired capabilities attributes. | ||
| You may also query keys providing a default value when those keys are nonexistent. | ||
There was a problem hiding this comment.
Does the browser drivers, like Chrome driver, has documentation what they return. If there is, it would be better to link in those documents.
There was a problem hiding this comment.
There is the Webdriver standard mentioned in doc (and Firefox is not following it completely, for platform and version).
There was a problem hiding this comment.
It would be better to link to the WebDriver documentation instead to trying to explain all in our documentation.
| Note: Firefox/Geckodriver is returning different names for '*platform*' and '*version*'. | ||
| It returns: '*platformName*' and '*browserVersion*'. | ||
There was a problem hiding this comment.
The example explains how RF dictionary works, which is best to leave to RF documentation. Perhaps providing few common attributes, like browserName would be enough.
| New in SeleniumLibrary 3.0. | ||
| """ | ||
| |
There was a problem hiding this comment.
Write return DotDic (self.browser.desired_capabilities)
There was a problem hiding this comment.
Also remove not required new lines.
| @@ -0,0 +1,16 @@ | |||
| class AttrDict(dict): | |||
There was a problem hiding this comment.
Should we rename this in same way as RF DotDic.
There was a problem hiding this comment.
Although code is simple write few unit test might be good idea.
There was a problem hiding this comment.
This, I don't agree. I only added AttrDict because of RF 2.8.7 compatibility. It is a temporary workaround.
The original PR had unit tests, but they will not be necessary because it is temporary.
There was a problem hiding this comment.
There already is some temporary code, waiting for the minimum RF version to raise high enough and they also have unit tests. So having something as temporally and not writing test for it, is not valid reason. Also it would lift the burden from the acceptance test, that one does not have to verify that value is in the right key in the acceptance test level.
There was a problem hiding this comment.
Couldn't this just wait until we drop RF 2.8.7 support? I don't think this can make it to 3.0 anyway.
| | ||
| Create Webdriver With Bad Keyword Argument Dictionary | ||
| [Documentation] Invalid arguments types | ||
| Run Keyword And Expect Error kwargs must be a dictionary. |
There was a problem hiding this comment.
Having own test suite is better idea.
| Run Keyword And Expect Error kwargs must be a dictionary. | ||
| ... Create Webdriver Firefox kwargs={'spam': 'eggs'} | ||
| | ||
| Get Browser Capabilities Using Dictionary |
There was a problem hiding this comment.
Could you change the test that one test tests only a one thing. Also get rid of the stuff which is not anymore valid like the [Setup].
| ... Create Webdriver Firefox kwargs={'spam': 'eggs'} | ||
| | ||
| Get Browser Capabilities Using Dictionary | ||
| [Documentation] Gets current browser capabilities from WebDriver |
There was a problem hiding this comment.
If the doc repeats the test name, then it's not useful and must be removed.
| ${capabilities}= Get Browser Capabilities | ||
| Log Capabilities = ${capabilities} | ||
| ${browser}= Normalize Browser Name | ||
| Should Match ${capabilities['browserName']} ${browser} |
There was a problem hiding this comment.
Change tests that it checks keys and does not care about the value. Because value changes between browser and perhaps Selenium version.
Test in the unit test level that correct value is in correct key.
Returns a dictionary with keys acessible as attributes of the current browser capabilities. Includes examples with logging of Common Browsers capabilities. Fixes unit tests when running on robotframework 2.8.7. Adds unit tests for DotDict. Moves acceptance tests to its own test suite.
HelioGuilherme66 force-pushed the get_browser_capabilities branch from 24baf59 to 63527f1 Compare HelioGuilherme66 force-pushed the get_browser_capabilities branch from 8b2175d to 69a7950 Compare because next test suite is expecting an opened browser.
| I consider Done the changes to this PR. |
| For this to be impelemented in SeleniumLibrary2.7 i had to make a small change to the existing code. return self._current_browser().get_log(log_type) |
| @HelioGuilherme66 would you be still interested to contribute this to the project? Because Robot Framework minimum version is raised to 3.0.4 in the SeleniumLibrary 4.0 release and therefore we could delete that dotdic implementation from this PR. Or can we close this PR? |
| Closing due inactivity. |
4 participants
Returns a dictionary with keys accessible as attributes of the current browser capabilities.
Includes examples with logging of Common Browsers capabilities.
This PR overrides PR #482 and fixes #980.