Skip to content

Fix buildfarm build of rmf_traffic_editor packages#495

Merged
luca-della-vedova merged 4 commits intomainfrom
luca/fix_uncrustify
May 29, 2024
Merged

Fix buildfarm build of rmf_traffic_editor packages#495
luca-della-vedova merged 4 commits intomainfrom
luca/fix_uncrustify

Conversation

@luca-della-vedova
Copy link
Copy Markdown
Member

@luca-della-vedova luca-della-vedova commented May 29, 2024 •
edited
Loading

Bug fix

Fix uncrustify test on buildfarm.

Fixed bug

Trying to address buildfarm failure.

My hypothesis is that rmf_utils was not specified as a test dependency, hence the config file would not be found and when running uncrustify it would actually not pass our custom config file (when looking for ament_uncrustify in the log, there is no -c [path to file] argument). Hence my proposed fix is to just add it as a test dependency.
The other line is just a cleanup in the CMakeLists since it still refers to a path in galactic

@luca-della-vedova
Remove mention of galactic from uncrustify
c1d472f 
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
@luca-della-vedova
Add rmf_utils as a test dependency
82d7722 
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
@luca-della-vedova luca-della-vedova changed the title Remove mention of galactic from uncrustify Remove mention of galactic from uncrustify, add test dependency May 29, 2024
@luca-della-vedova luca-della-vedova marked this pull request as ready for review May 29, 2024 14:31
@luca-della-vedova luca-della-vedova enabled auto-merge (squash) May 29, 2024 14:35
@luca-della-vedova
Copy link
Copy Markdown
Member Author

luca-della-vedova commented May 29, 2024 •
edited
Loading

Upon further investigation, it seems this would make the build from UNSTABLE to green, but the real problematic package is rmf_traffic_editor_assets:

Starting >>> rmf_traffic_editor_assets

----------------------------------------------------------------------
Ran 0 tests in 0.000s

NO TESTS RAN
--- stderr: rmf_traffic_editor_assets

----------------------------------------------------------------------
Ran 0 tests in 0.000s

NO TESTS RAN
---
Failed   <<< rmf_traffic_editor_assets [0.00s, exited with code 5]

Seems to be due to a change in upstream Python that changed the return code for NO TESTS RAN from 0 to an error code to detect cases where tests were supposed to be ran python/cpython#98904

@luca-della-vedova luca-della-vedova marked this pull request as draft May 29, 2024 14:49
auto-merge was automatically disabled May 29, 2024 14:49

Pull request was converted to draft

@luca-della-vedova
Add workflow for assets
0773dae 
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
@luca-della-vedova
Change rmf_traffic_editor_assets to CMake build
df611c4 
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
@luca-della-vedova luca-della-vedova changed the title Remove mention of galactic from uncrustify, add test dependency Fix buildfarm build of rmf_traffic_editor packages May 29, 2024
@luca-della-vedova
Copy link
Copy Markdown
Member Author

luca-della-vedova commented May 29, 2024 •
edited
Loading

df611c4 changed the rmf_traffic_editor_assets from a Python to a CMake package, I added the package to the CI and it was red before this commit green with it so it can also act as a regression test for (unlikely) future regressions

@luca-della-vedova luca-della-vedova marked this pull request as ready for review May 29, 2024 15:36
@luca-della-vedova luca-della-vedova enabled auto-merge (squash) May 29, 2024 15:36
@luca-della-vedova luca-della-vedova merged commit 19784b0 into main May 29, 2024
@luca-della-vedova luca-della-vedova deleted the luca/fix_uncrustify branch May 29, 2024 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Yadunund Yadunund Yadunund approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@luca-della-vedova @Yadunund