Continue to allow access to _table_element_finder class

@pekkaklarck

Per discussion with @pekkaklarck on the Slack group...

Essential Issue

Having created some Python helper functions, using the Selenium2Library, I have been able to leverage some of the private methods to accomplish some useful things for my testing framework. An example below shows a function get_column_number which used to be able to return the column number of whatever (valid) column name was passed in:

from robot.libraries.BuiltIn import BuiltIn

def _get_sel2lib():
    """== Gets Current Selenium 2 Instance from Robot Framework ==
    
    - Uses the Robot Framework API to get an object of the current Selenium 2 instance.
    - This is an internal helper function only. 
    
    == Calling ==
    
    | *Args* | [none] | |
    | *Returns* | ``object`` | An object instance of the current Selenium 2 library.
    | *Raises* | [none] | |
    """
    return BuiltIn().get_library_instance('Selenium2Library')

def get_column_number(table_locator, col_text, loglevel='INFO'):
    """== Returns Number of Specified Column ==
    
    Returns the number of the first column found which contains the ``col_text`` string.

    - Does not require an exact match; if there is a column named ``Foobar`` and you input ``Foo`` then it will match.
    
    === Calling ===
    
    | *Args* | ``table_locator`` (str) | The table locator containing the column whose number you desire. |
    |        | ``col_text`` (str) | The string of the column you want to locate. |
    | *Returns* | ``int`` | Number of column containing string ``col_name``. |
    | *Raises* | AssertionError | If ``col_name`` cannot be found in the table specified. |

    === Example in Robot ===

    | ${column_number} =    Get Column Number    xpath=//*[@id="deviceListHeader"]    Unsafe
    """
    sel2lib = _get_sel2lib()
    locators = sel2lib._table_element_finder._parse_table_locator(table_locator, 'header')
    found_it = False
    for locator in locators:
        elements = sel2lib._element_finder.find(sel2lib._current_browser(), locator)
        counter = 0
        for element in elements:
            counter += 1
            if col_text in element.text:
                output = counter
                found_it = True
                break
    if found_it is False:
        sel2lib.log_source(loglevel)
        raise AssertionError("No column containing '{}' found in the table identified via {}!".format(col_text, table_locator))
    return output

With the new SeleniumLibrary, I get an error: AttributeError: 'SeleniumLibrary' object has no attribute '_table_element_finder' as that structure doesn't exist as-is. I do notice, however, that in tableelementfinder.py there is a method named find_by_header which I could leverage if I could access it.

General Request

Provide a means to access some of these methods to get more granular access to some of the inner workings of the Selenium library.

@GLMeece

Using the private methods is always a risky, because they are private and not meant to used publicly. But also I have to admit that the old architecture was not very good and many things which where private should have been public. Let's think what can be done with your problem and can we make some of the methods public. Ed example had a need for the ElementFinder class, because he did want to overwrite some methods to better support AngularJS extension.

@GLMeece could you find/grep your code and see which private methods you need from the S2L? Then we would have a better picture which private methods you actually need and can better decide how to solve the problem. As a quick thought, perhaps we could do something similar than what we did do with the ElementFinder. One can access the class from the library instance, it is here: https://github.com/robotframework/SeleniumLibrary/blob/master/src/SeleniumLibrary/__init__.py#L262
If we provide access in similar way, would it satisfy your needs?

But please look into your code and tell if there is a need for other private methods.

@aaltat

Thanks @aaltat - I only recently returned to the project containing these methods and encountered the issues indicated. I will look through my code and see where my particular pain points are, and then try to extrapolate to more general "rules" as to what makes sense to expose publicly. Thanks for your thoughts and help on this.

Yeah, we should have a goal to expose stuff needed when extending the library publicly. This includes adding tests to ensure we don't break the public API in the future.

We also can add, and have added, some of the old private APIs back for the time being with appropriate deprecation warning. I doubt that's feasible with all APIs, though.

@GLMeece

By looking that code snippet, if SL library __init__ would have this self.table_element_finder = TableElementFinder(self), then @GLMeece could access the methods he needs from the SL and TableElementFinder. Only thing he would have to change in his code would be _table_element_finder to table_element_finder.

@GLMeece

The #912 proposes a fix for the problem.

Also did notice that @GLMeece needs access to the ElementFinder but that is already available in the public API. Only thing need which needs change, in @GLMeece side, is sel2lib._element_finder.find(sel2lib._current_browser(), locator) to sel2lib._element_finder.find(locator)

OK, never mind my P.R. Yours does effectively the same thing plus a test.

@aaltat

FWIW, my revised function using the P.R. that @aaltat created:

def get_column_number(table_locator, col_text, loglevel='INFO'):
    """== Returns Number of Specified Column ==
    
    Returns the number of the first column found which contains the ``col_text`` string.

    - Does not require an exact match; if there is a column named ``Foobar`` and you input ``Foo`` then it will match.
    
    === Calling ===
    
    | *Args* | ``table_locator`` (str) | The table locator containing the column whose number you desire. |
    |        | ``col_text`` (str) | The string of the column you want to locate. |
    | *Returns* | ``int`` | Number of column containing string ``col_name``. |
    | *Raises* | AssertionError | If ``col_name`` cannot be found in the table specified. |

    === Example in Robot ===

    | ${column_number} =    Get Column Number    xpath=//*[@id="deviceListHeader"]    Unsafe
    """
    sel2lib = _get_sel2lib()
    found_it = False
    counter = 0
    table = sel2lib.table_element_finder.find(table_locator)
    if table:
        head_cols = table.find_elements_by_xpath("./thead/tr/th")
        for header_col in head_cols:
            counter += 1
            element = sel2lib.element_finder.find(header_col)
            if col_text in element.text:
                output = counter
                found_it = True
                break
    else:
        sel2lib.log_source(loglevel)
        raise AssertionError("No table using selector '{}' found on the current page.".format(table_locator))
    if found_it is False:
        sel2lib.log_source(loglevel)
        raise AssertionError("No column containing '{}' found in the table identified via {}!".format(col_text, table_locator))
    return output

@GLMeece

@GLMeece did you have time to take a look in your code base to find out other possible API which you may need?

The new get_column_number function is so much cleaner when it doesn't need to touch _private_parts!

That function actually looks pretty generic. Stuff like this could be easily added to SL itself.

@aaltat

@aaltat - I was working late at work on this particular issue as well as some others, so I was not able to finish examining the rest of the code base. However, taking a look at it now - most of what I have cared about has been for table-related things, so for my limited use-cases, this enhancement is good! 👏

@pekkaklarck

BTW, @pekkaklarck - you are more than welcome to modify what I've done and include it in 🤖 I'd be happy to make even small contributions to a project (and the people behind it) that has helped me so much!

@pekkaklarck

We did discuss with @pekkaklarck about the code quality in the TableElementFinder and because the code is, hmm interesting (in a bad way), we decided to make the TableElementFinder as a private method for the library. This mean that after 3.0.0 release, there will be some thinking, designing and refactoring (read rewrite) of the table keywords. Therefore we do not want the TableElementFinder as public method in the library, more details for the future enhancement can be fond in #914

Did forget to mention that keyword API is not intended to change, but the changes are planned in the underlying code.

I realize that RF and related libraries are for testing and assertions. Regardless, there's still a lot of utility that can be leveraged (and often has to be leveraged) by a test engineer. That is why I wrote this and similar routines in the first place.

A typical scenario: depending on various factors (user login, environmental factors, changing design), the specific column number might change quite rapidly, but the text itself might be much more stable. As a result, I've had to create this utility so I might know which column to look at for a given value.

Even if the code is "less than optimal", having access to some of these helper functions, such as those listed in tableelementfinder.py seem very helpful - to me, at least. 🙂

Created issue #915 where discussion about the table keywords should continue.

@GLMeece

The PR #912 is now merged in master, which provides a fix for this problem. Therefore closing the issue.

@GLMeece if you want to try out the changes, then you can install from source, example like this: https://stackoverflow.com/questions/15268953/how-to-install-python-package-from-github

@GLMeece

@GLMeece It's fine that libraries are used programmatically by other libraries or any other code. We just need to make the related APIs public and stable. Whenever you are using something that _starts_with_an_underscore there's a risk that the API changes in the future. It's great that Python doesn't prevent you from using such private APIs, but you should acknowledge the risk. Probably the best idea would be submitting an issue and proposing making that part of the API stable. In the end the best solution might then be adding a new keyword that exposes the functionality directly to everyone.

@aaltat

@aaltat Shouldn't this issue actually be marked as duplicate of #882? Not sure do we need both of these in the release notes.

Ah I did remember that one, difficult question. I did already put this issue number in PR text and the PR is already merged. It is true that this kind of duplicate of the #882. If we want to mark this as duplicate, then we should put the information in #882.

Marking this as a dupe for #882 would basically mean removing milestone and adding duplicate label.

@GLMeece

@GLMeece when where looking at the library documentation and the TableElementFinder class, we decided to refactor the TableElementFinder and ElementFinder classes. This was done to unify the locator strategy with table keywords to match with the rest of the library, PR in #921

This refactoring caused that some of the internal methods which your original code did rely on, do not anymore exist. Example the _parse_table_locator is removed and _search_in_locators arguments have changed (+also the logic too). But more details in #921.

Navigation Menu

Repository navigation

Continue to allow access to _table_element_finder class #911

Essential Issue

General Request

2 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Continue to allow access to _table_element_finder class #911

Description

Essential Issue

General Request

Activity

aaltat commented on Sep 21, 2017

GLMeece commented on Sep 21, 2017

pekkaklarck commented on Sep 21, 2017

aaltat commented on Sep 21, 2017

aaltat commented on Sep 21, 2017

GLMeece commented on Sep 22, 2017

GLMeece commented on Sep 22, 2017

aaltat commented on Sep 22, 2017

pekkaklarck commented on Sep 22, 2017

GLMeece commented on Sep 22, 2017

GLMeece commented on Sep 22, 2017

2 remaining items

aaltat commented on Sep 22, 2017

aaltat commented on Sep 22, 2017

GLMeece commented on Sep 22, 2017

aaltat commented on Sep 23, 2017

aaltat commented on Sep 23, 2017

pekkaklarck commented on Sep 23, 2017

pekkaklarck commented on Sep 23, 2017

aaltat commented on Sep 23, 2017

pekkaklarck commented on Sep 23, 2017

aaltat commented on Sep 26, 2017

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions