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

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

Merged
merged 4 commits into from
Mar 25, 2022

Conversation

j-rivero
Copy link
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.

…r negative zero

Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
@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
@osrf-triage osrf-triage added this to Inbox in Core development Feb 7, 2022
Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
@codecov
Copy link

codecov bot commented Feb 7, 2022

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.

@scpeters scpeters moved this from Inbox to In review in Core development Feb 7, 2022
Copy link
Contributor

@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.

src/OrientedBox_TEST.cc Outdated Show resolved Hide resolved
@jennuine jennuine mentioned this pull request Feb 16, 2022
7 tasks
Signed-off-by: Louise Poubel <louise@openrobotics.org>
@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
Core development automation moved this from In review to Done Mar 25, 2022
@chapulina chapulina deleted the fixes_for_testi386 branch March 25, 2022 02:00
@j-rivero j-rivero removed this from Done in Core development May 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏰 citadel Ignition Citadel 🏢 edifice Ignition Edifice 🏯 fortress Ignition Fortress Gazebo 1️1️ Dependency of Gazebo classic version 11
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants