Skip to content

Conversation

CoolSpy3
Copy link
Contributor

@CoolSpy3 CoolSpy3 commented Jan 12, 2025

Description
This PR fixes a potential race condition in the robot_window_html test. The linked issue mentioned that this test was failing. However, more interestingly, the recent-runs of this test have been passing (since ~Dec 17). No changes were made to the develop branch during this time.

My guess is that this failure was caused by the test not exiting the wb_robot_wwi_receive_text loop after calling wb_robot_wwi_send_text. If the robot window returns the response quickly, it is consumed by the first loop, causing the second loop to hang indefinitely. This would explain the intermittent failures.

This PR forces the loop to break after the send statement, fixing this case. This should hopefully also fix the test on 22.04.

It should be noted that, for me, the test passes normally, but fails in a headless environment (presumably due to the web browser not starting), so that could be contributing to the difficulties in the CI environment. My hope is that because the test has passed before, it should be functional here.

UPDATE: My theory appears to have been wrong. The failure still occurs here (although why it passes on develop remains a mystery. I managed to fix my local build in headless mode by switching off of the default snap install of Firefox, however, that same fix doesn't appear to work here. (In fact, the logs indicate that Firefox is not installed through snap on GitHub runners.)~~

UPDATE 2: Alright, it looks like a runner update was pushed at around the same time this test started passing on develop, so I'm going to guess that that's the culprit. However, it also appears that these changes still fail on master, but pass here. Perhaps it's due to the ongoing work to make develop compatible with 24.04? In any case, I've backed out most of my changes here. What remains is the following:

  1. The way the original test was written was somewhat confusing for me. I've rewritten some of the logic to hopefully make it clearer.
    • The test message also referenced complete_test, which briefly led me down a rabbit hole of investigating the ROS tests. I've fixed the message to reflect the correct information.
  2. Early on, I started running this test on Ubuntu 22.04. It's been consistently passing there, so I'll assume that whatever problem had led to it being disabled no longer exists. Thus, this PR reenables it.

UPDATE 3: Alright, now the test is consistently failing everywhere. I think it's due to xdg-open not being able to launch a browser in a headless environment. I was able to get around it by explicitly calling firefox -headless, but making that work here would require explicitly invoking Firefox in WbDesktopServices::openUrl, which seems like it would create more problems than it solves. Thus, I think the best course of action is to just disable the test in CI.

Related Issues
This pull-request fixes issue #6727.

@CoolSpy3 CoolSpy3 added test suite issue Test failure in the test suite CI test suite Start the test suite labels Jan 12, 2025
@CoolSpy3 CoolSpy3 changed the title Fix race condition in robot window html test Fix Race Condition in Robot Window HTML Test Jan 12, 2025
@CoolSpy3 CoolSpy3 changed the base branch from master to develop January 12, 2025 18:10
@CoolSpy3 CoolSpy3 force-pushed the fix-race-condition-in-robot-window-html-test branch from 41c62a6 to 1abb36a Compare January 12, 2025 18:16
@CoolSpy3 CoolSpy3 added test suite Start the test suite test distribution Start the distribution test and removed test suite Start the test suite labels Jan 12, 2025
@CoolSpy3 CoolSpy3 changed the title Fix Race Condition in Robot Window HTML Test Clarify Robot Window HTML Test Jan 12, 2025
@CoolSpy3 CoolSpy3 marked this pull request as ready for review January 13, 2025 00:27
@CoolSpy3 CoolSpy3 requested a review from a team as a code owner January 13, 2025 00:27
@CoolSpy3 CoolSpy3 removed the test distribution Start the distribution test label Jan 13, 2025
@CoolSpy3 CoolSpy3 changed the base branch from develop to master March 11, 2025 04:48
@CoolSpy3 CoolSpy3 force-pushed the fix-race-condition-in-robot-window-html-test branch from 1e5c3d5 to 7c2dbf9 Compare March 11, 2025 04:50
@CoolSpy3 CoolSpy3 force-pushed the fix-race-condition-in-robot-window-html-test branch 3 times, most recently from b2f9d8f to 34be4c9 Compare June 26, 2025 06:01
@CoolSpy3 CoolSpy3 force-pushed the fix-race-condition-in-robot-window-html-test branch from 34be4c9 to 8535cf9 Compare July 7, 2025 03:55
@CoolSpy3 CoolSpy3 force-pushed the fix-race-condition-in-robot-window-html-test branch from 63e3c55 to 969326e Compare July 7, 2025 05:48
@CoolSpy3 CoolSpy3 requested a review from omichel July 13, 2025 09:03
Copy link
Member

@omichel omichel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you.

@CoolSpy3 CoolSpy3 merged commit 78755a4 into cyberbotics:master Jul 13, 2025
22 checks passed
@CoolSpy3 CoolSpy3 deleted the fix-race-condition-in-robot-window-html-test branch July 13, 2025 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test suite issue Test failure in the test suite CI test suite Start the test suite
Development

Successfully merging this pull request may close these issues.

2 participants