[fix] Real time updates on visualizer view#257
Conversation
nemesifier
left a comment
There was a problem hiding this comment.
Probably you need to modify the CI or test requirements to install the channels branch which has the patch for the live server?
| "ENGINE": "openwisp_utils.db.backends.spatialite", | ||
| "NAME": os.path.join(BASE_DIR, "openwisp_network_topology.db"), | ||
| "TEST": { | ||
| "NAME": os.path.join(BASE_DIR, "test_openwisp_network_topology.db"), |
There was a problem hiding this comment.
Yeah, ChannelLiveServerTestCase can't run with an in-memory database.
|
When running tests in parallel with Django versions < 5, I encounter an error due to |
da4f489 to
425b480
Compare
nemesifier
left a comment
There was a problem hiding this comment.
We're almost there with this one.
The following test is too flaky in my environment:
[Retry] Retrying "openwisp_network_topology.tests.test_realtime.TestRealTime.test_node_link_update", attempt 1/5.
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
[Retry] Retrying "openwisp_network_topology.tests.test_realtime.TestRealTime.test_node_link_update", attempt 2/5.
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
[Retry] Retrying "openwisp_network_topology.tests.test_realtime.TestRealTime.test_node_link_update", attempt 3/5.
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
[Retry] Retrying "openwisp_network_topology.tests.test_realtime.TestRealTime.test_node_link_update", attempt 4/5.
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
[Retry] Retrying "openwisp_network_topology.tests.test_realtime.TestRealTime.test_node_link_update", attempt 5/5.
--------------------------------------------------------------------------------
I disabled the retry logic in opernwisp-utils to see the stacktrace:
ERROR: test_node_link_update (openwisp_network_topology.tests.test_realtime.TestRealTime)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/nemesis/.virtualenvs/openwisp2/lib/python3.10/site-packages/asgiref/sync.py", line 262, in __call__
return call_result.result()
File "/usr/lib/python3.10/concurrent/futures/_base.py", line 451, in result
return self.__get_result()
File "/usr/lib/python3.10/concurrent/futures/_base.py", line 403, in __get_result
raise self._exception
File "/home/nemesis/.virtualenvs/openwisp2/lib/python3.10/site-packages/asgiref/sync.py", line 302, in main_wrap
result = await awaitable
File "/home/nemesis/Code/openwisp/openwisp-network-topology/openwisp_network_topology/tests/test_realtime.py", line 190, in test_node_link_update
len(self.web_driver.execute_script("return graph.data;")["links"]),
File "/home/nemesis/.virtualenvs/openwisp2/lib/python3.10/site-packages/selenium/webdriver/remote/webdriver.py", line 528, in execute_script
return self.execute(command, {"script": script, "args": converted_args})["value"]
File "/home/nemesis/.virtualenvs/openwisp2/lib/python3.10/site-packages/selenium/webdriver/remote/webdriver.py", line 429, in execute
self.error_handler.check_response(response)
File "/home/nemesis/.virtualenvs/openwisp2/lib/python3.10/site-packages/selenium/webdriver/remote/errorhandler.py", line 232, in check_response
raise exception_class(message, screen, stacktrace)
selenium.common.exceptions.JavascriptException: Message: ReferenceError: graph is not defined
Stacktrace:
@moz-nullprincipal:{5659b005-1c36-40dd-a1c2-c1026854d042}:2:7
@moz-nullprincipal:{5659b005-1c36-40dd-a1c2-c1026854d042}:3:8
This is what I see if I use to stop the testing right before the failing line (with ipdb):
An empty tab, which of course doesn't yield the desired result. Can you please change the test to fix this issue? I suggest using ipdb to run the code line by line in this test and find a different way to implement it that doesn't cause flakyness.
There was a problem hiding this comment.
Is there a test for the non-admin visualizer though (which #250 is all about)?
|
LGTM! |
pandafy
left a comment
There was a problem hiding this comment.
@Dhanus3133 While debugging some issues in openwisp/openwisp-firmware-upgrader#320, I had some doubts.
| node_model = Node | ||
| link_model = Link | ||
| topology_model = Topology | ||
| application = import_string(getattr(settings, "ASGI_APPLICATION")) |
There was a problem hiding this comment.
Why do we need this? This is imported when the class is loaded. This may happen before ready for all app config is executed.
There was a problem hiding this comment.
It wasn't imported. I tried it.
openwisp-network-topology/openwisp_network_topology/tests/utils.py
Lines 20 to 37 in 20f8800
There was a problem hiding this comment.
What did you try?
My question was regarding application = import_string(getattr(settings, "ASGI_APPLICATION"))
In channels, I see the following code:
So, the test class will automatically get the application from the Django settings.
There was a problem hiding this comment.
Oh, I thought you were talking about models since GitHub highlighted those too. Yeah, it seems like we could have used it as well. I used this as a reference after seeing it in another test:
| self.org = org | ||
|
|
||
| async def _prepare(self, admin=True): | ||
| communicator = await self._get_communicator(self.admin_client, self.topology.pk) |
There was a problem hiding this comment.
What is the use of communicator in selenium tests? Aren't we making sending/receiving data using the browser directly?
There was a problem hiding this comment.
To verify WebSocket changes. No, we are updating directly using queries.
There was a problem hiding this comment.
Aren't we ensuring that the UI updates after receiving data from the websocket?
There was a problem hiding this comment.
Aren't we ensuring that the UI updates after receiving data from the websocket?
Dhanus here verified both:
- data is received as expected
- the UI updates as expected, with some limitations here due to the use of canvas but we're checking the JS data structures have updated as expected
I think it's a solid approach.
Closes #250 #251