-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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_duo does not work with All-in-One components #1933
Comments
It works fine with in-lined components in test functions, see https://gist.github.com/lucassus/32256b4cc4587d3188d56c3a83ca0bf4 |
Update: It works fine when I used a shared dash app instance, see https://gist.github.com/lucassus/892e593366e04ca207722c73af6a34f2 |
This is a bug with Lines 1378 to 1381 in 6ee2328
|
@T4rk1n - What do you think a solution would be for this? If you have something in mind, I'd be happy to give it a go at implementing it. My initial thoughts was to duplicate the |
There could be another callback map, something like ORIGINAL_CALLBACK_MAP, inserting the callbacks into it at the same time as the app callback_map at line 1378. Then you can take the difference of the the original callback and the app callback map and insert those missing. |
This is a tricky one. I'm looking at #2062 removing the code that clears the global stores, but I'll comment here so we can decide on the right direction to go. For running an app normally (not during tests) I don't think it matters either way - the only case this would affect is multiple dash apps running side-by-side in the same Python process. That's a discouraged pattern where you just shouldn't be using During testing it's more complicated, at least in principle, because we're repeatedly creating and destroying apps, and we don't want these apps affecting each other. If we think I can think of two directions to go with this:
from dash.testing import stash_callbacks
from my_aio_component import SomeButton
my_aio_callbacks = stash_callbacks()
def test_my_aio_component(dash_duo):
my_aio_callbacks.apply()
app = Dash()
app.layout = html.Div(SomeButton()) In this scenario, clearing the global store after loading an AIO component and before any test case runs is important because otherwise the first test case will get these callbacks no matter what. If that first test case happens to also reimport this component, these callbacks will be duplicated and there will be an error. Curious what @T4rk1n and @AnnMarieW think, but I guess after all that I'm inclined toward the first option: merge #2062 as is, then at some point modify |
@alexcjohnson It looks like this was added to make the The last scenario looks like a nice solution. 👍 |
Ah, thanks @AnnMarieW - I can't see the specific failures over there anymore, and the tests in #2062 do pass, but maybe just because of how they test files are currently being partitioned for parallel execution, or the order the tests happen to run. We do have a I do like the simplicity of "callbacks defined within a test are local to that test, callbacks defined at the top level are global" from the first option, and I bet we can do this fairly easily in |
Also Line 1491 in 34fd848
|
I thought it was clearing the callbacks for tests where the app is restarted, maybe the tests got refactored but I think there is a valid use case. Example: import pytest
from dash import Dash, html, callback, Input, Output
app = Dash(__name__)
app.layout = html.Div([
html.Button("click", id="input"),
html.Div(id="output")
])
@callback(Output("output", "children"), Input("input", "n_clicks"))
def on_click(n_clicks):
return n_clicks
@pytest.mark.parametrize("counter", [(1,), (2,), (3,)])
def test_usage_params(dash_duo, counter):
dash_duo.start_server(app)
dash_duo.wait_for_element("#input").click()
dash_duo.wait_for_text_to_equal("#output", "1") |
|
Oh sorry - you’re saying restarting the same app defined outside the test using |
User apps may be imported and started across multiple tests, but then we have |
I see, I was thinking about the use case of creating an app purely for test purposes but you're referring to testing a specific app. Yes, that's a very important use case. One way around that is what we do in our docs (that's the old public repo but still valid) - start the app once (session scope) and only restart the browser with each test: @pytest.fixture(scope="session")
def doc_server():
with ProcessRunner() as server:
server(raw_command="python index.py", port=8060, start_timeout=60)
yield server
@pytest.fixture
def dash_doc(doc_server, request):
with Browser(...) as browser:
browser.server_url = doc_server.url
yield browser In addition to avoiding the problem here, this skips the overhead of multiple app starts - huge for big apps like the docs which can take 30 sec to start up, if you're running maybe 100 tests on them! So we could probably do something to make that pattern easier to replicate. Leaving that aside, how do we handle the three cases here?
I don't see any one rule that would satisfy all of these, but we can take them one at a time:
|
@alexcjohnson - I'll give these changes a go in #2062 and see if there are any issues. Reading the above discussions and the different parts that have been effected by this, is there any way to write some tests for this to pick up any future issues changes like this could have? |
I think we'd just want to create some tests covering the three cases I mentioned above. Case 3 (defining callbacks via |
Describe the bug
It seems that dash testing utils don't work property with All-in-One components. For instance, given the following test scenario:
The first run
test_some_button_first_time
is perfectly fine. Buttest_some_button_second_time
results withselenium.common.exceptions.TimeoutException: Message: text -> Loaded not found inside element within 10s
error.I ran it with
--pause
option and in the DevTools I noticed that for the second test the update callback_dash-update-component
is not being triggered (see the attached screenshots).Moreover, if I run
test_some_button_second_time
separately or if I comment out the first test everything is fine.For some reason callbacks are fired only for the first test.
Expected behavior
It seems that
dash_duo
does not work with@callback
for all-in-one components.Screenshots
The text was updated successfully, but these errors were encountered: