Skip to content

ENH: Changed test macros to copy result - #1709

Draft
JDuffeyBQ wants to merge 1 commit into
developfrom
topic/test_macro_enh
Draft

ENH: Changed test macros to copy result#1709
JDuffeyBQ wants to merge 1 commit into
developfrom
topic/test_macro_enh

Conversation

@JDuffeyBQ

@JDuffeyBQ JDuffeyBQ commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

Fixes #1704

SIMPLNX_RESULT_REQUIRE_VALID and SIMPLNX_RESULT_REQUIRE_INVALID now assign the input Result<T> to avoid macro duplication when the argument is an expression (may copy or move depending on input type).

For tests, I think the following typical form should still be preferred since it allows for easy access to the Result in the debugger. This PR mainly serves as a guard against unintended behavior.

  auto executeResult = filter.execute(dataStructure, args);
  SIMPLNX_RESULT_REQUIRE_VALID(executeResult.result);

Comment thread test/UnitTestCommon/include/simplnx/UnitTest/UnitTestCommon.hpp
@JDuffeyBQ
JDuffeyBQ force-pushed the topic/test_macro_enh branch from 886296b to e38ad27 Compare August 21, 2026 15:26
- Wrapped inside do while block for formatting
- Now check that the argument to the macros is actually a Result

Signed-off-by: Jared Duffey <jared.duffey@bluequartz.net>
@JDuffeyBQ
JDuffeyBQ force-pushed the topic/test_macro_enh branch from e38ad27 to ce4ba55 Compare August 21, 2026 20:16
@JDuffeyBQ

Copy link
Copy Markdown
Collaborator Author

After thinking about this change, one issue with this method is that it makes the error message when a REQUIRE statement fails is less helpful. There will still be the line number but for quick diagnosing of errors the message helped.

Because of that it might be better to just add a compile time check that only allows lvalues to prevent the macro expansion from running the expression multiple times. I've tested a version that does that locally now and there are currently only 3 locations that fail. All of which are quick fixes.

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.

SIMPLNX_RESULT_REQUIRE_VALID evaluates its argument 4+ times, silently re-running filter calls

1 participant