Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(autoware_test_utils): porting autoware_test_utils from universe to core #172

Merged
merged 13 commits into from
Feb 25, 2025

Conversation

JianKangEgon
Copy link
Contributor

Description

We are porting autoware_test_utils to autoware.core, and this PR adds the package to core.

Related links

Parent Issue:

  • Link

How was this PR tested?

Notes for reviewers

None.

Interface changes

None.

Effects on system behavior

None.

@JianKangEgon JianKangEgon added the run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) label Jan 18, 2025
@JianKangEgon JianKangEgon self-assigned this Jan 18, 2025
Copy link

github-actions bot commented Jan 18, 2025

Thank you for contributing to the Autoware project!

🚧 If your pull request is in progress, switch it to draft mode.

Please ensure:

@JianKangEgon
Copy link
Contributor Author

This pr depends on the new ported package "autoware_utils": autowarefoundation/autoware_utils#23 and "autoware_internal_msgs": autowarefoundation/autoware_internal_msgs#45

Copy link

@mounireom1 mounireom1 left a comment

Choose a reason for hiding this comment

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

jncr gen

Copy link
Member

@youtalk youtalk left a comment

Choose a reason for hiding this comment

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

If this package is only used during testing, as its name suggests, it shouldn’t be in the common directory. I think it would be better to create a tests directory and move it there just like the demos directory.

Copy link

codecov bot commented Jan 24, 2025

Codecov Report

Attention: Patch coverage is 37.78071% with 471 lines in your changes missing coverage. Please review.

Project coverage is 36.94%. Comparing base (4cb18f5) to head (c1f0fae).
Report is 49 commits behind head on main.

Files with missing lines Patch % Lines
...ng/autoware_test_utils/src/autoware_test_utils.cpp 4.73% 161 Missing ⚠️
...autoware_test_utils/test/test_mock_data_parser.cpp 11.11% 3 Missing and 157 partials ⚠️
...g/autoware_test_utils/src/topic_snapshot_saver.cpp 0.00% 83 Missing ⚠️
...sting/autoware_test_utils/src/mock_data_parser.cpp 78.18% 47 Missing and 13 partials ⚠️
...nclude/autoware_test_utils/autoware_test_utils.hpp 83.33% 1 Missing and 4 partials ⚠️
...are_test_utils/test/test_autoware_test_manager.cpp 90.00% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main     #172       +/-   ##
===========================================
- Coverage   78.75%   36.94%   -41.82%     
===========================================
  Files          11       29       +18     
  Lines         193     1164      +971     
  Branches       73      523      +450     
===========================================
+ Hits          152      430      +278     
- Misses         11      533      +522     
- Partials       30      201      +171     
Flag Coverage Δ *Carryforward flag
differential 29.00% <37.78%> (?)
total 80.89% <ø> (+2.14%) ⬆️ Carriedforward from 6364a19

*This pull request uses carry forward flags. Click here to find out more.

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

Copy link
Member

@youtalk youtalk left a comment

Choose a reason for hiding this comment

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

#172 (review)

If this package is only used during testing, as its name suggests, it shouldn’t be in the common directory. I think it would be better to create a tests directory and move it there just like the demos directory.

@JianKangEgon Please move directory according to my comment.

JianKangEgon and others added 6 commits February 7, 2025 10:23
Signed-off-by: JianKangEgon <egon.kang@autocore.ai>
Signed-off-by: JianKangEgon <egon.kang@autocore.ai>
Signed-off-by: NorahXiong <norah.xiong@autocore.ai>
Signed-off-by: NorahXiong <norah.xiong@autocore.ai>
@liuXinGangChina
Copy link
Contributor

Fatih san @xmfcx and mits san @mitsudome-r

I notice that you have create a test folder , for this "autoware_test_utils"

Should I put "autoware_test_utils" under core/testing/ , alongside package autoware_testing that you created?

Best regards

心刚

@xmfcx
Copy link
Contributor

xmfcx commented Feb 14, 2025

@liuXinGangChina Hello 👋

Should I put "autoware_test_utils" under core/testing/ , alongside package autoware_testing that you created?

Yes, please 👍

@liuXinGangChina
Copy link
Contributor

@liuXinGangChina Hello 👋

Should I put "autoware_test_utils" under core/testing/ , alongside package autoware_testing that you created?

Yes, please 👍

got it, Fatih san

have a nice weekend

心刚

Signed-off-by: mitsudome-r <ryohsuke.mitsudome@tier4.jp>
Signed-off-by: mitsudome-r <ryohsuke.mitsudome@tier4.jp>
@mitsudome-r
Copy link
Member

mitsudome-r commented Feb 18, 2025

@liuXinGangChina I have moved the package to testing in this commit: a3074ea

Copy link
Member

@youtalk youtalk left a comment

Choose a reason for hiding this comment

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

autoware_pyplot was finally ported.

Copy link
Member

@youtalk youtalk left a comment

Choose a reason for hiding this comment

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

I applied my suggestions 2024e7f by myself. LGTM

@mitsudome-r
Copy link
Member

mitsudome-r commented Feb 20, 2025

Waiting for: autowarefoundation/autoware.universe#10175
-> Merged

@mitsudome-r
Copy link
Member

mitsudome-r commented Feb 20, 2025

@xmfcx xmfcx merged commit b27283b into autowarefoundation:main Feb 25, 2025
25 of 27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci)
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

7 participants