Use absolute imports for generated local type support#261
Conversation
Signed-off-by: Lidang-Jiang <[email protected]>
|
@Lidang-Jiang did you use AI to write all/some of this code or open this PR? |
|
Thanks for checking. Yes, I used OpenAI Codex (GPT-5) to help investigate the issue, prepare the code and test changes, and draft/update this PR description. I updated the PR body with a Generative AI disclosure section as well. |
|
@InvincibleRMC could you review this? |
| if(BUILD_TESTING) | ||
| find_package(ament_cmake_pytest REQUIRED) | ||
|
|
||
| find_package(builtin_interfaces REQUIRED) |
There was a problem hiding this comment.
I will run CI to double check. But I'm pretty sure this won't work since this would cause a circular package order. We might need to make another pr before this splitting the tests into there own rosidl_generator_py_tests or something to avoid this.
There was a problem hiding this comment.
Yeah CI complains about being unable to topologically order the package after adding builtin_interfaces.
There was a problem hiding this comment.
Good catch, fixed in 3dd9104. I removed the builtin_interfaces test dependency and moved the regression coverage to a template-level pytest so this no longer affects package ordering.
There was a problem hiding this comment.
Confirmed, fixed in 3dd9104 by removing the generated builtin_interfaces test interfaces and the CMake/package.xml dependency that caused the topological cycle.
There was a problem hiding this comment.
Also addressed in 3dd9104: the package-order failure is gone because the PR no longer adds builtin_interfaces to rosidl_generator_py CMake/package test dependencies.
|
Pulls: #261 |
Move the absolute import regression coverage from generated test interfaces to a template-level pytest so rosidl_generator_py does not need to depend on builtin_interfaces during testing. Signed-off-by: Lidang Jiang <[email protected]>
Summary
Fixes #257.
import builtin_interfaces.msg.check_fieldsassertions.builtin_interfaces/Duration, without adding a package-level test dependency onbuiltin_interfaces.Did you use Generative AI?
Yes. I used OpenAI Codex (GPT-5) to help investigate the issue, prepare the code and test changes, and draft/update this PR description. I reviewed the changes and ran the local tests listed below before opening the PR.
Before / After
Before
Command:
Output:
After
Command:
rg -n "builtin_interfaces\\.msg|from builtin_interfaces\\.msg" \ build_pr/rosidl_generator_py/rosidl_generator_py/rosidl_generator_py/msg/_duration.py \ build_pr/rosidl_generator_py/rosidl_generator_py/rosidl_generator_py/msg/_duration_array_sequence.pyOutput:
Tests
Command:
Output:
Command:
Output: