Pytest: wait for Mermaid to render

[evnchn]

Motivation

Infuriating to wake up to a red pipeline in my fork. Consider https://github.com/evnchn/nicegui/actions/runs/18337127046

Implementation

This pull request enhances the test coverage and reliability for Mermaid diagram rendering in both test_markdown.py and test_mermaid.py by adding explicit assertions for node content and display. The changes ensure that key nodes are present or absent as expected after user interactions.

Improvements to test assertions for Mermaid diagram rendering:

• Added checks in tests/test_markdown.py to assert that 'Node_A' and 'Node_C' are present or absent as appropriate after setting new content, and that these nodes are displayed in the rendered output.
• Included assertions after clicking 'Create Mermaid' in tests/test_markdown.py to verify that both 'Node_A' and 'Node_B' are present and displayed.
• Enhanced tests/test_mermaid.py by adding assertions to confirm that 'Node_A' and 'Node_C' are present and have the correct class, and that 'Node_A' is absent after content changes.

HUMAN NOTE: By the "checks" I mean screen.should_contain which has a built-in retry mechanism. Check #5232 (comment) and

nicegui/nicegui/testing/screen.py

Lines 195 to 210 in c5c9dfe

	def find(self, text: str) -> WebElement:
	"""Find the element containing the given text."""
	try:
	query = f'//*[not(self::script) and not(self::style) and text()[contains(., "{text}")]]'
	# HACK: repeat check after a short delay to avoid timing issue on fast machines
	for _ in range(5):
	element = self.selenium.find_element(By.XPATH, query)
	try:
	if element.is_displayed():
	return element
	except StaleElementReferenceException:
	pass
	self.wait(0.2)
	raise AssertionError(f'Found "{text}" but it is hidden')
	except NoSuchElementException as e:
	raise AssertionError(f'Could not find "{text}"') from e

Progress

• I chose a meaningful title that completes the sentence: "If applied, this PR will..."
• The implementation is complete.
• Pytests have been added (or are not necessary).
• Documentation has been added (or is not necessary).

[Copilot]

Pull Request Overview

This PR addresses test flakiness in Mermaid diagram rendering by adding explicit wait times to ensure asynchronous rendering completes before assertions are performed.

• Adds screen.wait(1.0) calls after Mermaid content updates to prevent race conditions
• Improves test stability for CI/CD pipelines by allowing sufficient rendering time
• Targets specific points where Mermaid diagrams are dynamically updated or initially loaded

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
tests/test_mermaid.py	Added wait calls after initial page load and content update to ensure Mermaid rendering completes
tests/test_markdown.py	Added wait call after Mermaid content update to prevent assertion failures on slow rendering

[falkoschindler]

Thanks for looking into this flaky test, @evnchn! It also caught my attention a few days ago.

But I think your new delays are irrelevant to the specific problem. If the test fails, it also happens in test_markdown.py in test_markdown_with_mermaid() at the line

assert screen.selenium.find_element(By.XPATH, '//span[p[contains(text(), "Node_A")]]').is_displayed()

That's above the new delay this PR is adding. Before this line there is already a delay which I recently increased to 1 second in 3b2d6a0. Unfortunately this didn't help.

So it remains unclear how to fix this problem. We could further increase the duration, but this adds to the overall time it takes to run all tests. Maybe calling screen.wait(1.0) after screen.should_contain('Mermaid') could help, so we don't spend our precious waiting time while the page is still loading and rendering hasn't started yet?

[evnchn]

        screen.open('/')
        screen.wait(1.0)  # wait for Mermaid to render
        screen.should_contain('Mermaid')
        assert screen.find_by_tag('svg').get_attribute('id') == f'{m.html_id}_mermaid_0'
>       assert screen.selenium.find_element(By.XPATH, '//span[p[contains(text(), "Node_A")]]').is_displayed()
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Indeed the waits, as inserted, do not fix the issue. Because the issue surfaces even with the waits just like how I inserted it. My apologies for the 🤡.

Consider my latest approach at 8a65143, which uses screen.should_contain (has a built-in retry mechanism) to probe for the Node (not the text "Mermaid", since that shows up always instantly as part of the Markdown) before trying to do screen.selenium.find_element on it. Not updated...

Note that with #4722 and #4692 we no longer flash-render the Mermaid raw syntax text since we use the Mermaid API directly. I wrote it so I know it.

---

Well, as with pipeline stuff, there's no better way but to try out different and basically fire shots into the dark (tests always pass locally...). It's more "merge it if it doesn't break existing tests, then pray that it isn't flaky".

@falkoschindler Two things though, for another PR:

• Maybe we can submit the artifact of the screenshot, if any, so that we see them visually? I don't think theres a way with GitHub Actions right now, and it will be an important tool.
• How about using https://pypi.org/project/pytest-retry/ everywhere? Doesn't it sound more convincing that a test is failing, when it has failed 10 times in a row?

[evnchn]

https://github.com/zauberzeug/nicegui/actions/runs/18405814901/job/52445352379

Huh... So Mermaid is broken when we have a failing test...

[falkoschindler]

Very interesting! This confirms my suspicion that we shouldn't simply repeat tests that don't pass on first try.

[evnchn]

We will have to re-think, if that's the case.

• This PR: Still safe to merge. Maybe with my more tolerant logic Mermaid loads.
• Use pytest-rerunfailures to re-run flaky Mermaid tests #5236: If the purpose of adding reruns is merely to shove problems under a rug, we will say no to it.
• pytest-xdist support (parallelized tests!) #5249: If the purpose of adding reruns is to make parallelized tests work, then maybe we can say yes, but we may need to run the the test suite in a fully non-parallelized manner before release / at midnight, where we are not actively waiting for the pipeline to inform our coding.

[falkoschindler]

@evnchn Do we really need _wait_for_xpath_displayed, or is it sufficient to swap the first wait and the first should_see?

[evnchn]

@falkoschindler AFAIK screen.should_contain, although internally tries a couple of times, errors out when the element is available but hidden, and so my _wait_for_xpath_displayed is the only way to try a couple of times on the "Node_A" which could be hidden for a while until it's displayed.