Repository navigation
Externally useful attributes should be declared public and documented. #882
Description
Old Selenium2Library has several attributes that are useful externally from custom libraries and extensions:
_run_on_failure_keyword
_speed_in_secs
_timeout_in_secs
_implicit_wait_in_secs
_current_browser()
We should indicate that using these externally is OK by removing the leading underscore. The current WebDriver instance should also be accessible as a normal attribute (i.e. without calling a method like current_browser()) and we should name the attribute webdriver to make it explicit what it actually is. I would also remove unnecessary _in_secs postfixes. In other words, I would like to have these attributes instead:
run_on_failure_keyword
speed
timeout
implicit_wait
webdriver
Renaming these attributes is obviously a backwards incompatible change. We should thus add propertys that enable the old functionality but emit deprecation warnings. These propertys can then be removed in 3.1 or later.
UPDATES:
-
We used
browserattribute instead ofwebdriveruntil 3.0 RC 1. -
In 3.0 RC 1 we added also methods
find_elementandfind_elementsdirectly to the library itself. These methods form the recommended public API to find elements for external tools.
Activity
Make public API public. robotframework#882
_browser -> browser robotframework#882
pekkaklarck commentedon Sep 2, 2017
This is mostly fixed if #884 is merged. The remaining decisions to make are:
Should deprecated attributes be unit tested somehow? Shouldn't be too hard, but adding tests for features that are going to be removed pretty soon feels a bit like waste.
What do to with
_browsers?Cleanup (#884)
aaltat commentedon Sep 6, 2017
pekkaklarck commentedon Sep 9, 2017
If we aren't going to add more tests and we'll keep
_browsersas-is, isn't this issue done?aaltat commentedon Sep 10, 2017
Yes it's. Closing
6 remaining items
Make (table_)element_finder private.
boakley commentedon Oct 21, 2017
I agree with @aaltat - driver and browser both seem better than webdriver. I would prefer you stick with browser since the code is already in release candidate status and public interfaces shouldn‘t be changing.
pekkaklarck commentedon Oct 21, 2017
We aren't in RC yet. In earlier versions this attribute was
_browserand thus it wasn't really public. The reason I'd call itwebdriveris that it is an instance ofWebDriverand tbe idea is that external users can use its functionality as needed. That name would make the situation explicit even without much documentation.boakley commentedon Oct 21, 2017
Add 'find_element(s)' to public API (#965)
aaltat commentedon Oct 23, 2017
Adding sphinx style definitions helps users of the methods and IDE's to provide auto completions for arguments and return values.
paramtoparamandtypeto better support Sublime Anaconda pluginaaltat commentedon Oct 23, 2017
I prefer using
driver, because that is defacto way of referring to theWebDriverinstance.I agree that making changes this late in release pipeline is causing problems for the end users who have extended SL with their own libraries. I am one of those users, too. But I also agree that the old public API, or lack of it, is also a big problem. I hope that we can now implement a better public API which allows extending the library in more easier and cleaner way.
aaltat commentedon Oct 23, 2017
Did forget to say that the plan is to keep some of the old non public API interface in place, to ease the transition from S2L 1.8.0 to SL 3.0.0. If I recall correctly, @boakley requirements for the old API are pretty much the same as mine. But I know that if users have used
ElementFinder,TableElementFinderor some other internal functionality, the code is not anymore backwards compatible.[-]Externally useful attributes should be declared public and not start with an underscore[/-][+]Externally useful attributes should be declared public and documented.[/+]aaltat commentedon Oct 31, 2017
Document public properties and
find_elementswith sphinx documentation. At least document the types and if method document return types.Rename `browser` attribute to `driver` (#976)
aaltat commentedon Nov 14, 2017
The sphinx documentation is done in #994