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.

Sign up for GitHub

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

Jump to bottom

testRadToHMSStr fails on x86 #2460

Closed
DarthGandalf opened this issue May 23, 2022 · 9 comments
Closed

testRadToHMSStr fails on x86 #2460

DarthGandalf opened this issue May 23, 2022 · 9 comments
Assignees
@alex-w
Labels
importance: low Small problem, rarely visible, no crash
Milestone
0.22.2

Comments

@DarthGandalf
Copy link
Contributor

DarthGandalf commented May 23, 2022 •
edited
Loading

Expected Behaviour

Unit tests should pass

Actual Behaviour

Unit test fails:

FAIL!  : TestConversions::testRadToHMSStr() 'expectedHMS==hms' returned FALSE. (2.74889 radians = 10h30m00.01s (expected 10h30m00.00s))
   Loc: [/var/tmp/portage/sci-astronomy/stellarium-0.22.1/work/stellarium-0.22.1/src/tests/testConversions.cpp(942)]

Steps to reproduce

  1. use 32 bit x86 system
  2. cmake; make; ctest

System

  • Stellarium version: 0.22.1
  • Operating system: Gentoo Linux
  • Graphics Card: irrelevant
  • Screen type (if applicable): irrelevant

Logfile

build.log

https://bugs.gentoo.org/847064
@gzotti
Copy link
Member

gzotti commented May 23, 2022

Uh, where do I find a 32bit x86 today?

@gzotti gzotti added the importance: low Small problem, rarely visible, no crash label May 23, 2022
@DarthGandalf
Copy link
Contributor Author

32 bit software can be run on 64 bit (x86_64) CPU as well

You can e.g. download "32 bit" from https://www.debian.org/distrib/ or "x86" from https://www.gentoo.org/downloads/ and run it on a VM with a usual x86_64 virtual CPU (or even physical machine)

@gzotti
Copy link
Member

gzotti commented May 23, 2022

Sure I can setup an x86 VM and start compiling. Or maybe you can fix this issue please and send a PR? May be faster.

@alex-w
Copy link
Member

alex-w commented May 24, 2022

I found something strange in the code of conversion :-/

alex-w added a commit that referenced this issue May 24, 2022
@alex-w
Fixed something strange in radToHMSStr and radToHMSStrAdapt convertors
802065e 
probably ancient bug (?), maybe related to issue #2460
@alex-w alex-w added this to the 0.22.2 milestone May 24, 2022
@DarthGandalf
Copy link
Contributor Author

@alex-w that fixed the test, thanks!

@gzotti

Or maybe you can fix this issue please and send a PR?

My best idea was to randomly replace double with long double, which is probably not a good solution, even if it would work

DarthGandalf added a commit to DarthGandalf/gentoo that referenced this issue May 24, 2022
@DarthGandalf
sci-astronomy/stellarium: fix test on x86
098476d 
Patch from Stellarium/stellarium#2460

Closes: https://bugs.gentoo.org/847064
Package-Manager: Portage-3.0.30, Repoman-3.0.3
Signed-off-by: Alexey Sokolov <alexey+gentoo@asokolov.org>
@DarthGandalf DarthGandalf mentioned this issue May 24, 2022
Closed
@alex-w alex-w self-assigned this May 25, 2022
@DarthGandalf
Copy link
Contributor Author

Looks like this broke amd64:

FAIL!  : TestConversions::testRadToHMSStr() 'expectedHMS==hms' returned FALSE. (0.08727 radians = 0h20m-0.0s (expected 0h20m00.0s))                                                                                                                                               
   Loc: [/mnt/portagetmp/portage/sci-astronomy/stellarium-0.22.1-r1/work/stellarium-0.22.1/src/tests/testConversions.cpp(938)]

@DarthGandalf DarthGandalf reopened this May 25, 2022
@alex-w
Copy link
Member

alex-w commented May 25, 2022

This is weird, but I see where we can get negative zero.

alex-w added a commit that referenced this issue May 25, 2022
@alex-w
Fixed conversion tool radToHms(): avoiding possible negative zero for…
dd65fdf 
… value of seconds (see #2460)
@alex-w alex-w closed this as completed May 25, 2022
DarthGandalf added a commit to DarthGandalf/gentoo that referenced this issue May 26, 2022
@DarthGandalf
sci-astronomy/stellarium: fix test on x86
1f116a4 
Patches from Stellarium/stellarium#2460

Closes: https://bugs.gentoo.org/847064
Package-Manager: Portage-3.0.30, Repoman-3.0.3
Signed-off-by: Alexey Sokolov <alexey+gentoo@asokolov.org>
gentoo-bot pushed a commit to gentoo/gentoo that referenced this issue May 26, 2022
@DarthGandalf @juippis
sci-astronomy/stellarium: fix test on x86
15111b1 
Patches from Stellarium/stellarium#2460

Closes: https://bugs.gentoo.org/847064
Package-Manager: Portage-3.0.30, Repoman-3.0.3
Signed-off-by: Alexey Sokolov <alexey+gentoo@asokolov.org>
Closes: #25626
Signed-off-by: Joonas Niilola <juippis@gentoo.org>
@alex-w alex-w added the state: fixed The bug has been fixed label May 26, 2022
@github-actions
Copy link

Hello @DarthGandalf! Please check the fresh version (development snapshot) of Stellarium:
https://github.com/Stellarium/stellarium-data/releases/tag/weekly-snapshot

gzotti pushed a commit that referenced this issue Jun 8, 2022
@alex-w @gzotti
Fixed something strange in radToHMSStr and radToHMSStrAdapt convertors
031cefa 
probably ancient bug (?), maybe related to issue #2460
gzotti pushed a commit that referenced this issue Jun 8, 2022
@alex-w @gzotti
Fixed conversion tool radToHms(): avoiding possible negative zero for…
92d5d31 
… value of seconds (see #2460)
@alex-w alex-w removed the state: fixed The bug has been fixed label Jul 7, 2022
@github-actions
Copy link

github-actions bot commented Jul 7, 2022

Hello @DarthGandalf! Please check the latest stable version of Stellarium:
https://github.com/Stellarium/stellarium/releases/latest

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@alex-w alex-w

Labels
importance: low Small problem, rarely visible, no crash
Projects
None yet
Milestone
0.22.2
Development

No branches or pull requests
3 participants
@alex-w @DarthGandalf @gzotti