Skip to content

Fixes for tests on i386: relax SphericalCoordinates and workaround for negative zero#374

Merged
chapulina merged 4 commits intoign-math6from
fixes_for_testi386
Mar 25, 2022
Merged

Fixes for tests on i386: relax SphericalCoordinates and workaround for negative zero#374
chapulina merged 4 commits intoign-math6from
fixes_for_testi386

Conversation

@j-rivero
Copy link
Copy Markdown
Contributor

@j-rivero j-rivero commented Feb 7, 2022

Two changes to make i386 testing build happy after ign-math 6.10:

  • OrientedBox_TEST: the problem appeared before, when comparing strings that comes from a zero value in maths, the optimizations in some platforms return a negative zero making the comparison to fail. The workaround is to compare the two substrings around the zero.
  • SphericalCoordinates_TEST: some float operations in i386 does not return the exact number but a really close one. Change the test to reflect this reality.

Fixes for tests on i386: relax SphericalCoordinates and workaround fo…
83422a9 
…r negative zero

Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
@j-rivero j-rivero requested a review from scpeters as a code owner February 7, 2022 13:58
@github-actions github-actions bot added Gazebo 1️1️ Dependency of Gazebo classic version 11 🏢 edifice Ignition Edifice 🏯 fortress Ignition Fortress 🏰 citadel Ignition Citadel labels Feb 7, 2022
Fix linter
dce9277 
Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
@codecov
Copy link
Copy Markdown

codecov bot commented Feb 7, 2022 •
edited
Loading

Codecov Report

Merging #374 (908fcc1) into ign-math6 (3b3e7ab) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           ign-math6     #374   +/-   ##
==========================================
  Coverage      99.65%   99.65%           
==========================================
  Files             67       67           
  Lines           6380     6380           
==========================================
  Hits            6358     6358           
  Misses            22       22

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3b3e7ab...908fcc1. Read the comment docs.

mjcarroll
mjcarroll reviewed Feb 7, 2022
View reviewed changes
Copy link
Copy Markdown

@mjcarroll mjcarroll left a comment

Choose a reason for hiding this comment

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

One comment to maybe make this an easier fix.

@jennuine jennuine mentioned this pull request Feb 16, 2022
Merged
7 tasks
@chapulina
Don't use zero in test
cea52b5 
Signed-off-by: Louise Poubel <louise@openrobotics.org>
@chapulina
Merge branch 'ign-math6' into fixes_for_testi386
908fcc1
@chapulina chapulina enabled auto-merge (squash) March 25, 2022 01:06
@chapulina chapulina disabled auto-merge March 25, 2022 02:00
@chapulina chapulina merged commit 4fbd3c8 into ign-math6 Mar 25, 2022
@chapulina chapulina deleted the fixes_for_testi386 branch March 25, 2022 02:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mjcarroll mjcarroll mjcarroll left review comments

@scpeters scpeters Awaiting requested review from scpeters scpeters is a code owner

+1 more reviewer

@chapulina chapulina chapulina approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

🏰 citadel Ignition Citadel 🏢 edifice Ignition Edifice 🏯 fortress Ignition Fortress Gazebo 1️1️ Dependency of Gazebo classic version 11

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@j-rivero @mjcarroll @scpeters @chapulina @osrf-triage