Skip to content

Split build and tests to reduce number of URSim startups - #1910

Open
urfeex wants to merge 66 commits into
UniversalRobots:mainfrom
urfeex:refactor_ci
Open

urfeex wants to merge 66 commits into
UniversalRobots:mainfrom
urfeex:refactor_ci

Conversation

@urfeex

@urfeex urfeex commented Jul 24, 2026 •

Copy link
Copy Markdown
Member

Our current approach towards running integration tests doesn't scale well. We're spending a lot of time setting up and tearing down simulators.

This PR aims at

  • geting more immediate feedback
  • using less test resources in total
  • being able to run tests on more target platforms

Note

Medium Risk
Reworks the entire industrial_ci pipeline (workspace cache, committed Docker image, test labels). Failures would mainly affect CI reliability and integration coverage, not production robot control.

Overview
Splits binary/semi-binary CI into a build job and parallel integration jobs so URSim is started once per PolyScope family instead of once per test.

The build job still uses industrial_ci, runs everything except tests labelled ursim, then caches the workspace and commits the CI image. Three follow-on jobs restore that image and run ctest -L ursim against UR30 latest, UR15 PolyScope X 10.12.0 (excluding dashboard_g5), and CB3 UR10 3.15.8. Workflows also get PR concurrency cancellation.

Integration tests no longer launch URSim themselves. They wait on wait_robot_booted.py, take UR_CI_ROBOT_IP / UR_CI_UR_TYPE from the environment, reset simulator state between cases, and skip features that are unavailable on CB3 or PolyScope X. Dashboard tests detect the PolyScope family at runtime instead of parametrizing URSim versions. The old in-repo DockURSim image is removed in favor of start_ursim.sh plus stored program folders.

Reviewed by Cursor Bugbot for commit 97927a2. Bugbot is set up for automated code reviews on this repo. Configure here.

@codecov

codecov Bot commented Jul 24, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 27.62%. Comparing base (1b121b7) to head (848e02a).
⚠️ Report is 634 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##            main    #1910       +/-   ##
==========================================
+ Coverage   3.59%   27.62%   +24.03%     
==========================================
  Files         13       41       +28     
  Lines        947     5622     +4675     
  Branches     152      660      +508     
==========================================
+ Hits          34     1553     +1519     
- Misses       843     3997     +3154     
- Partials      70       72        +2     
Flag Coverage Δ
unittests 27.62% <ø> (+24.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot AI balanced review requested due to automatic review settings November 3, 2026 20:11

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

There are 3 total unresolved issues (including 2 from previous reviews).

Fix All in Cursor

Reviewed by Cursor Bugbot for commit 37f6c13. Configure here.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Neither client-library dependency source installs the referenced wait_robot_booted.py executable, blocking driver startup and causing integration CI to exit before running tests.

1 open issue
1 resolved since last review

Review effort: Balanced

Comment thread ur_robot_driver/test/test_common.py Resolved
Copilot AI balanced review requested due to automatic review settings November 4, 2026 06:08

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 Needs a closer look

The test helper points to a wait_robot_booted.py executable that the pinned client-library dependency does not install, preventing driver startup in shared-URSim and robot-model tests.

1 open issue

Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings November 4, 2026 06:15

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 Needs a closer look

The test helper now relies on a waiter executable that official ur_client_library installations do not provide, causing driver startup and robot-model tests to fail in dependency configurations not covered by the Rolling override.

1 open issue

Review effort: Balanced

It has been there before this PR, so we can always remove it later.
Copilot AI balanced review requested due to automatic review settings November 4, 2026 13:17

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

The unconditional install entry still references the deleted scripts/wait_robot_booted.py, causing ur_robot_driver installation to fail even when integration tests are disabled.

1 open issue
1 resolved since last review

Review effort: Balanced

install(PROGRAMS
scripts/tool_communication.py
scripts/wait_for_robot_description
scripts/wait_robot_booted.py

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this file needed?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Consider gunzip the urp programs, installation and variables

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this files needed?

IoStatusInterface,
)

URSIM_CONTAINER = "ursim"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please address

type: git
url: https://github.com/UniversalRobots/Universal_Robots_Client_Library.git
version: master
url: https://github.com/urfeex/Universal_Robots_Client_Library.git

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we still need this workaround, now that UniversalRobots/Universal_Robots_Client_Library#615 is merged?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants