Skip to content

Line2: increase test coverage (backport #807) - #829

Merged
caguero merged 1 commit into
ign-math6from
mergify/bp/ign-math6/pr-807
Aug 25, 2026
Merged

Line2: increase test coverage (backport #807)#829
caguero merged 1 commit into
ign-math6from
mergify/bp/ign-math6/pr-807

Conversation

@mergify

@mergify mergify Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

🦟 Bug fix

Improve test coverage of Line2 class

Summary

This increases test coverage based on suggestions from Gemini. After observing some windows C4244 compiler warnings, I fixed them in 2a719e4:

  • Fix warnings in Set(double, double, double, double) by wrapping with static_cast<T>
  • Fix warning in Length() by using the simpler implementation from Line3.hh wrapped with static_cast<T>:

public: T Length() const
{
return this->pts[0].Distance(this->pts[1]);
}

Summary of test coverage improvements

  1. Set with Vector2 parameters: Verified setting start/end points using Vector2 instances.
  2. CrossProduct: Added explicit tests for both Line2 and Vector2 cross products. Expect CrossProduct of parallel lines is 0.
  3. OnSegment and Within: Tested points on segment, collinear points outside bounds, non-collinear points within bounding box, and endpoints.
  4. Intersect Branch Coverage:
    • Parallel collinear non-overlapping line segments.
    • Collinear lines where only the second endpoint lies on the segment.
    • Line extensions that intersect outside X or Y bounding ranges.
    • Intersect overload taking no output point.
  5. Operators: Expanded operator== and operator!= self-comparison checks.
  6. Template Types: Added test cases for Line2i (int) and Line2f (float).

Backport Policy

  • This is safe to backport to the following versions:
    • Jetty
    • Ionic
    • Harmonic
    • Fortress
  • This should not be backported
  • I am not sure
  • Other (fill in yourself)

Checklist

  • Signed all commits for DCO
  • Added a screen capture or video to the PR description that demonstrates the fix (as needed)
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • Updated Bazel files (if adding new files). Created an issue otherwise.
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers
  • Was GenAI used to generate this PR? If so, make sure to add "Assisted-by" to your commits. (See this policy for more info.)

Assisted-by: Gemini 3.6 Flash

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by and Generated-by messages.

Backports: If this is a backport, please use Rebase and Merge instead.


This is an automatic backport of pull request #807 done by [Mergify](https://mergify.com).

@mergify

mergify Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

Cherry-pick of 268039c has failed:

On branch mergify/bp/ign-math6/pr-807
Your branch is up to date with 'origin/ign-math6'.

You are currently cherry-picking commit 268039c.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   src/Line2_TEST.cc

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   include/gz/math/Line2.hh
	both modified:   src/python_pybind11/test/Line2_TEST.py

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@mergify mergify Bot added the conflicts label Aug 25, 2026
@mergify mergify Bot added the conflicts label Aug 25, 2026
@mergify mergify Bot mentioned this pull request Aug 25, 2026
19 tasks
@github-actions github-actions Bot added the 🏯 fortress Ignition Fortress label Aug 25, 2026
@ahcorde ahcorde closed this Aug 25, 2026
@github-project-automation github-project-automation Bot moved this from Inbox to Done in Core development Aug 25, 2026
@caguero caguero reopened this Aug 25, 2026
@github-project-automation github-project-automation Bot moved this from Done to Inbox in Core development Aug 25, 2026
@caguero
caguero force-pushed the mergify/bp/ign-math6/pr-807 branch from 998ecac to bfae060 Compare August 25, 2026 19:12
@codecov

codecov Bot commented Aug 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.85%. Comparing base (5b2522a) to head (bfae060).
⚠️ Report is 14 commits behind head on ign-math6.

Additional details and impacted files
@@              Coverage Diff              @@
##           ign-math6     #829      +/-   ##
=============================================
- Coverage      99.38%   97.85%   -1.54%     
=============================================
  Files             75       75              
  Lines           7029     7582     +553     
=============================================
+ Hits            6986     7419     +433     
- Misses            43      163     +120     

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

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

Assisted-by: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Carlos Agüero <caguero@honurobotics.com>
@caguero
caguero force-pushed the mergify/bp/ign-math6/pr-807 branch from bfae060 to 28a6008 Compare August 25, 2026 19:42
@caguero caguero removed the conflicts label Aug 25, 2026
@github-project-automation github-project-automation Bot moved this from Inbox to In review in Core development Aug 25, 2026
@caguero
caguero merged commit 85a1d7d into ign-math6 Aug 25, 2026
8 checks passed
@caguero
caguero deleted the mergify/bp/ign-math6/pr-807 branch August 25, 2026 20:06
@github-project-automation github-project-automation Bot moved this from In review to Done in Core development Aug 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🏯 fortress Ignition Fortress

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants