Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Dash Testing: wait_for_text_to_equal may incorrectly succeed when used with text "None" #2818

Closed
emilykl opened this issue Mar 27, 2024 · 1 comment · Fixed by #2823

Comments

@emilykl
Copy link
Contributor

emilykl commented Mar 27, 2024

Describe your context

  • replace the result of pip list | grep dash below
dash                            2.16.1         
dash-core-components            2.0.0
dash-dangerously-set-inner-html 0.0.2
dash-flow-example               0.0.5
dash-html-components            2.0.0
dash-table                      5.0.0
dash-testing-stub               0.0.2

Describe the bug

When wait_for_text_to_equal is used to wait for the text "None", the function will often succeed even when you would reasonably expect it to fail.

I think this is part of the reason why the regression in #2733 wasn't caught by the tests.

This behavior is demonstrated by the following test case:

import dash
from dash import html


def test_wait_for_text_to_equal_none(dash_duo):
    app = dash.Dash(__name__)
    app.layout = html.Div(id="my-div", children="Hello world")
    dash_duo.start_server(app)
    dash_duo.wait_for_text_to_equal("#my-div", "None", timeout=4)

Expected behavior

The test should fail because the contents of the #my-div div are never equal to None or "None".

Actual behavior

The test passes.

Explanation

This happens because wait_for_text_to_equal checks not only the text content of the element, but also the value of the value attribute. (see here).

If value is not defined we get a value of None, which is then converted to a string and therefore matches the string "None".

So dash_duo.wait_for_text_to_equal("#my-div", "None") always succeeds unless the target element has a defined value.

Proposed solutions

IMO the cleanest solution would be to modify wait_for_text_to_equal to check only the element's text, and add a new function wait_for_value_to_equal which checks the value (or a generalized wait_for_attr_to_equal function). This would break backwards compatibility.

Alternatively we could have wait_for_text_to_equal ignore value if value is not defined, or issue a warning when used with the text "None".

@TillerBurr
Copy link
Contributor

TillerBurr commented Mar 28, 2024

I was just coming to look into this further, but looks like I was beaten to it.

Looking at the function, I think there's a another solution. My thought would be to change

dash/dash/testing/wait.py

Lines 106 to 113 in f7f8fb4

def __call__(self, driver):
try:
elem = self._get_element(driver)
logger.debug("text to equal {%s} => expected %s", elem.text, self.text)
return (
str(elem.text) == self.text
or str(elem.get_attribute("value")) == self.text
)

to

def __call__(self, driver): 
    try:  
        elem = self._get_element(driver) 
        logger.debug("text to equal {%s} => expected %s", elem.text, self.text) 
        value = elem.get_attribute("value")
        value = str(value) if value is not None else None
        return ( 
            str(elem.text) == self.text 
            or value == self.text 
        ) 

That way, the overall functionality of wait_for_text_to_equal doesn't change (except for the "None" case). The only downside is that it would pass if you use dash_duo.wait_for_text_to_equal("#my-div", None, timeout=4), but I don't know how often that would come up.

There is also an issue with wait_for_contains_text. The test

from dash import html
import dash


def test_wait_for_contains_text_none(dash_duo):

    app = dash.Dash(__name__)
    app.layout = html.Div(id="my-div", children="Hello world")
    dash_duo.start_server(app)
    dash_duo.wait_for_contains_text("#my-div", "No", timeout=4)

also passes, but similarly, there is no value and "No" is not in "Hello world".

There are potential issues with wait_for_contains_class and wait_for_class_to_equal, but I imagine those would be far less common.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants