Skip to content

Conversation

jsantoso91
Copy link
Contributor

@jsantoso91 jsantoso91 commented May 14, 2025

Fix for #1660 and follow up to #1658.

I created helper methods to call configure()/activate() and check expected state (for success and failure) afterwards.
Further replace instances of on_configure() and on_activate() with the said methods.

===

Contributions via pull requests are much appreciated. Before sending us a pull request, please ensure that:

  1. Limited scope. Your PR should do one thing or one set of things. Avoid adding “random fixes” to PRs. Put those on separate PRs.
  2. Give your PR a descriptive title. Add a short summary, if required.
  3. Make sure the pipeline is green.
  4. Don’t be afraid to request reviews from maintainers.
  5. New code = new tests. If you are adding new functionality, always make sure to add some tests exercising the code and serving as live documentation of your original intention.

To send us a pull request, please:

  • Fork the repository.
  • Modify the source; please focus on the specific change you are contributing. If you also reformat all the code, it will be hard for us to focus on your change.
  • Ensure local tests pass. (colcon test and pre-commit run (requires you to install pre-commit by pip3 install pre-commit)
  • Commit to your fork using clear commit messages.
  • Send a pull request, answering any default questions in the pull request interface.
  • Pay attention to any automated CI failures reported in the pull request, and stay involved in the conversation.

Copy link
Contributor

mergify bot commented May 14, 2025

This pull request is in conflict. Could you fix it @jsantoso91?

Copy link

codecov bot commented May 14, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.26%. Comparing base (30473a5) to head (6bb374f).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1682      +/-   ##
==========================================
+ Coverage   85.25%   85.26%   +0.01%     
==========================================
  Files         143      143              
  Lines       13794    13794              
  Branches     1194     1194              
==========================================
+ Hits        11760    11762       +2     
+ Misses       1636     1633       -3     
- Partials      398      399       +1     
Flag Coverage Δ
unittests 85.26% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...ive_controller/test/test_diff_drive_controller.cpp 94.49% <100.00%> (-0.10%) ⬇️
..._controllers/test/test_gpio_command_controller.cpp 99.02% <100.00%> (+0.01%) ⬆️
...r_broadcaster/test/test_gps_sensor_broadcaster.cpp 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jsantoso91 jsantoso91 marked this pull request as ready for review May 14, 2025 16:32
@jsantoso91 jsantoso91 marked this pull request as draft May 14, 2025 16:39
@jsantoso91
Copy link
Contributor Author

I am looking for feedback before proceeding to edit other tests with similar pattern.

Copy link
Contributor

This PR is stale because it has been open for 45 days with no activity. Please tag a maintainer for help on completing this PR, or close it if you think it has become obsolete.

@github-actions github-actions bot added stale and removed stale labels Jun 30, 2025
Copy link
Contributor

This PR is stale because it has been open for 45 days with no activity. Please tag a maintainer for help on completing this PR, or close it if you think it has become obsolete.

@github-actions github-actions bot added stale and removed stale labels Aug 18, 2025
@jsantoso91 jsantoso91 force-pushed the testfix/call_activate_and_configure_on_controller branch from 3761a92 to 5b9f14c Compare September 9, 2025 18:57
@jsantoso91 jsantoso91 marked this pull request as ready for review September 9, 2025 19:53
@jsantoso91
Copy link
Contributor Author

Hello, the test results look clean, could I get this PR reviewed? @christophfroehlich @bmagyar
Thanks!

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