Stellarium / stellarium

Stellarium is a free GPL software which renders realistic skies in real time with OpenGL. It is available for Linux/Unix, Windows and macOS. With Stellarium, you really see what you can see with your eyes, binoculars or a small telescope.
https://stellarium.org
GNU General Public License v2.0
7.71k stars 821 forks source link

qt6: TestJavaScripting::testColor() fails #2739

Closed DarthGandalf closed 2 years ago

DarthGandalf commented 2 years ago

Expected Behaviour

Unit tests pass

Actual Behaviour

TestJavaScripting::testColor() fails Describe or maybe attach a screenshot?

PASS   : TestJavaScripting::testV3f()
Script:
var f1 = new Color(0.5, 0.4, 0.3);
f1.getR().toFixed(1)+','+f1.getG().toFixed(1)+','+f1.getB().toFixed(1)

Script:
var f3 = new Color('#00ffff');
f3.toRGBString()

QWARN  : TestJavaScripting::testColor() "Could not convert argument 0 at"
QWARN  : TestJavaScripting::testColor()      "%entry@:1"
QWARN  : TestJavaScripting::testColor() Passing incomatible arguments to signals is not supported.
FAIL!  : TestJavaScripting::testColor() 'result == expect' returned FALSE. (var f3 = new Color('#00ffff');
f3.toRGBString()
=[r:1, g:1, b:1])
   Loc: [/mnt/portagetmp/portage/sci-astronomy/stellarium-1.0/work/stellarium-1.0/src/tests/testJavaScripting.cpp(254)]
PASS   : TestJavaScripting::cleanupTestCase()
Totals: 4 passed, 1 failed, 0 skipped, 0 blacklisted, 3ms

Steps to reproduce

System

Logfile

If possible, attach the logfile log.txt from your user data directory. Look into the Guide for its location.

LastTest.log

DarthGandalf commented 2 years ago

Testing it further... It fails with qt5 too, if I use -DENABLE_SCRIPT_QML=yes. But in this case the test fails even more LastTest.log

The test passes with -DENABLE_SCRIPT_QML=no, but this option is not available with qt6

gzotti commented 2 years ago

-DENABLE_SCRIPT_QML should be false for Qt5. True with Qt5 is currently unsupported. This is one of the key differences between Qt5 and Qt6 versions. But yes, this is a new issue I also found yesterday, it's new with Qt6.4.

github-actions[bot] commented 2 years ago

Hello @DarthGandalf!

OK, developers can reproduce the issue. Thanks for the report!

tofilwiktor commented 2 years ago

I'm not so good with Qt/C++, so I'm likely wrong but the entire Color class in V3d.hpp is defined within an #ifdef ENABLE_SCRIPT_QML block, so shouldn't this test only run when ENABLE_SCRIPT_QML is defined? Instead the testColor() function is outside such a block. I'm unable to test this at the moment because Stellarium doesn't properly compile with Qt6.4 on Arch-based Linux distributions (see: #2709).

gzotti commented 2 years ago

Does current master still not compile on Arch? I moved to 6.4 on the day of your report, solved a mini issue and never had a problem since.

tofilwiktor commented 2 years ago

Oh, you made me realize, I downloaded the source code from releases instead of just cloning, then complained about an older issue persisting 🤦. I think I'm just too tired right now. I'll clone and try compiling again.

tofilwiktor commented 2 years ago

After some testing and reading about QML I realized that the bug is actually caused by passing hexColor by reference in the Color constructor because QML apparently doesn't support reference signal parameters. Changing that seems to fix the issue.

gzotti commented 2 years ago

Ah, thanks. Did not check yet.

github-actions[bot] commented 1 year ago

Hello @DarthGandalf!

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

github-actions[bot] commented 1 year ago

Hello @DarthGandalf!

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