Skip to content

Restore dropped assertion messages across the test suite - #4461

Merged
nickbianco merged 1 commit into
mainfrom
restore_assert_messages
Sep 28, 2026
Merged

nickbianco merged 1 commit into
mainfrom
restore_assert_messages

Conversation

@nickbianco

@nickbianco nickbianco commented Sep 24, 2026 •

Copy link
Copy Markdown
Member

A follow up based on feedback in #4458.

Brief summary of changes

  • Restores the test assertion messages lost in Remove ASSERT and replace usages with OPENSIM_ASSERT_ALWAYS #4458.
  • Drive-by: delete calibrated_pelvis_rot.osim which gets generated by testOpenSense.cpp, but is not a requirement to run the test. Also comments out the output_model_file property value in the test setup file to prevent further file generation.

Testing I've completed

Looking for feedback on...

Ran tests locally + CI.

CHANGELOG.md (choose one)

  • no need to update because...internal test updates.

Stack created with GitHub Stacks CLI • Give Feedback 💬


This change is Reviewable

@nickbianco
nickbianco added this pull request to stack #4462 September 24, 2026 16:14
@nickbianco
nickbianco force-pushed the restore_assert_messages branch from 538ffa4 to 11d9e9d Compare September 24, 2026 16:55
@nickbianco
nickbianco force-pushed the restore_assert_messages branch 2 times, most recently from c4f7b23 to cb21521 Compare September 24, 2026 16:59
@nickbianco
nickbianco marked this pull request as ready for review September 24, 2026 17:13

@adamkewley adamkewley left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

:lgtm:

@adamkewley reviewed 16 files and all commit messages, and made 2 comments.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on nickbianco).


OpenSim/Simulation/tests/testFrames.cpp line 137 at r1 (raw file):

            rod1.getMobilizedBodyIndex()
                    == offsetFrame->getMobilizedBodyIndex() &&
            "testPhysicalOffsetFrameOnBody(): incorrect MobilizedBodyIndex");

Is the function name necessary in the messages? Cant the assertion macro pull this out with FUNC or similar?

@nickbianco
nickbianco force-pushed the restore_assert_messages branch from cb21521 to 83460d3 Compare September 28, 2026 15:20
@nickbianco
nickbianco force-pushed the restore_assert_messages branch from 83460d3 to 3458807 Compare September 28, 2026 15:36
@nickbianco
nickbianco force-pushed the restore_assert_messages branch 2 times, most recently from 7ea3636 to b15c338 Compare September 28, 2026 16:01
@nickbianco
nickbianco force-pushed the restore_assert_messages branch from b15c338 to 0bfb76a Compare September 28, 2026 16:10

@adamkewley adamkewley left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

:lgtm:

@adamkewley reviewed 4 files and all commit messages, and made 1 comment.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on nickbianco).

Base automatically changed from convert_test_macros_to_catch to main September 28, 2026 19:07
@nickbianco
nickbianco force-pushed the restore_assert_messages branch from 0bfb76a to 064edfd Compare September 28, 2026 19:07
@nickbianco
nickbianco merged commit 2aaa04b into main Sep 28, 2026
6 checks passed
@nickbianco
nickbianco deleted the restore_assert_messages branch September 28, 2026 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants