ARMv7: libmscore tests 74% tests passed
This is the first time I ran the libmscore tests on my arch linux ARMv7 machine. I'm just reporting here that "74% tests passed, 12 tests failed out of 46", and that two of those (tst_midi & tst_note) were execeptions. I'm betting this is more the the unspecified char stuff. I'm going to look into tomorrow, but just making a note here, and attaching the ctest output:
Comments
The detailed output of the tests would be needed, like the one Travis generates, showing the diff between what the test got and what was expected.
Sorry, here is the output with diff by running "ctest --output-on-failure": ctest-output-ArchLinuxARMv7-commit-5a12dff_with-diff.txt
Note: I always get "libEGL warning: DRI2: failed to open rockchip (search paths /usr/lib/xorg/modules/dri)" whenever I run musescore on this ARM...it is only about my ARM's gpu, which shouldn't have any bearing on these tests...
In reply to Sorry, here is the output by ericfontainejazz
Indeed looks like signed vs. unsigned char problems, -1 turning into 255 on transposing etc.
See https://github.com/musescore/MuseScore/blob/master/libmscore/interval.h…
Seems those need to get turned into signed chars.
As you have a testbed ready, I'd rather leave a PR to you ;-)
In reply to Indeed looks like signed vs. by Jojo-Schmitz
ok...
sure enough I'm noticing transposing notes down does not work, while transposing up does!
In reply to ok... sure enough I'm by ericfontainejazz
and if you change those two to signed char?
In reply to and if you change those two by Jojo-Schmitz
tst_chordsymbol passed
In reply to and if you change those two by Jojo-Schmitz
and so did a bunch of others...now the only ones left are:
tst_midimappin
tst_importmidi
tst_capella_io
tst_guitarpro
So thanks that was a good eye to help narrow down...
In reply to and so did a bunch of by ericfontainejazz
I'd bet this baby needs to be signed too: https://github.com/musescore/MuseScore/blob/master/libmscore/durationty…
Edit: I take that back
In reply to I'd bet this baby needs to be by Jojo-Schmitz
that one didn't seem to fix any of the errors...
In reply to Indeed looks like signed vs. by Jojo-Schmitz
As I remember, a plain char is undefined and left up to the compiler implementer to decide whether to make it signed or unsigned (the implementer doesn't even have to be consistent about it!).
I believe gcc chose to always turn them signed, but I know from my profession that arm-compilers can go either way. Most of them have a compile flag to force chars to be signed or unsigned though. Depending on the number of plain char definitions you might want to try using that compile flag as the easy route to testing what you want to test.
In the long run, plain chars should all be replaced with their correctly signed type imo.
In reply to As I remember, a plain char by jeetee
Well, yes, you do remember correcly, almost, whether a char is signed or unsigned is implementation defined behavoir (so inconsistency is not allowed, it needs to be defined, one way or the other) and that's what we know since quite a while, at least since the first ARM raleted Problems cropped up some months ago.
And only those chars that matter need to get fixed, those where it does not, don't. The problem is to tell one form the other :-(
In reply to Well, yes, you do remember by Jojo-Schmitz
yes, exactly!
Also, does anyone know a good way to tell qt-creator to compile tests in parallel. I'm noticing on my quad-core machine, that appending -j4 to the tests doesn't really help out significantly, because the linking stage is unnecessary serializer, and because for each test, there are only two cpp files which need to be compiled, hence limiting concurrency to a max of 2, e.g.:
mtest/libmscore/timesig/CMakeFiles/tst_tools.dir/tst_tools.cpp.o- mtest/libmscore/timesig/CMakeFiles/tst_tools.dir/tst_tools_automoc.cpp.o
Now I could just open up 2 or 3 terminal windows and split up list of tests, so can compile with maximum parallelism. But I'm wondering if there is something I can specify in the qt-creator to do this...
changing this char to signed has fixed tst_midimapping failures: https://github.com/musescore/MuseScore/blob/5a12dff4765bc924273dbd4fb25…
In reply to changing this char to signed by ericfontainejazz
This change is definitly the right thing. I don't understand though how that related to midimapping, this it GuitarPro
In reply to This change is definitly the by Jojo-Schmitz
there was one midi test that involved reading a guitar pro file...test file #14 in midimapping.
Anyway, good news after rerunning all tests, that I now only have two failures:
38 - test_importmidi
39 - test_capella_io
In reply to there was one midi test that by ericfontainejazz
these are test_importmidi errors...I've been trying to figure out...no luck so far...
I've traced through the key signature assigining...and I haven't spooted anything unusual...
In reply to these are test_importmidi by ericfontainejazz
fyi, this is what the import midi 15-8 looks like on my ARMv7 ...the diff says error is wrong keysignature...
In reply to fyi, this is what the import by ericfontainejazz
while this is what it looks like in x86-64:
And this is what the reference looks like:
In reply to while this is what it looks by ericfontainejazz
I actually think the ARM version is more correct...since 5-flats key signature can handle all these notes... Although i suppose 3-flats key signature would actually be better, since would be the minimum number of flats in key signature... I'm going to look into the logic for detecting the key signature a little more. But what my belief is now is that this line:
https://github.com/musescore/MuseScore/blob/e2c12c016bf03b24cf456664861…
is producing machine-dependent code, because I believe that std::sort is sorting a few different equally-valued options, so there can be different orderings that are both correct. The function then returns element [0], which can thus be different. Anyway, I'm thinking that the test function should be changed so is not dependent on a particular marchine's sort ordering.
In reply to I actually think the ARM by ericfontainejazz
oh...actually I should investiage this suspect:
QDEBUG : TestImportMidi::metertimeSig15_8() ImportMidi: illegal key 254
In reply to oh...actually I should by ericfontainejazz
indeed, after carefully tracing through line by line with both x86-64 and armv7 computers side-by-side, I notice that the execution different exactly at that line before the that QDEBUG statement:
https://github.com/musescore/MuseScore/blob/master/mscore/importmidi/im…
That code need to both explicitly cast that uchar *data into a signed char *, and also needs to store it in a signed char (I've checked that just storing into an int doesn't actually handle the sign extension)... PR comming soon...
I'm at the point where everything except tst_capella_io passes: https://github.com/musescore/MuseScore/pull/2509
In reply to I'm at the point where by ericfontainejazz
all about pitch and tpc, esp. pitch may be yet another signed char issue, tcp might be a follow up error.
Hmm, maybe not. both _tpc and _pitch are int and read as int
In reply to all about pitch and tpc, esp. by Jojo-Schmitz
thanks Jojo. Indeed it was pitch. Pitch of CNote: https://github.com/musescore/MuseScore/pull/2509/files#diff-ab0da389d41…
ok, I've just double-checked all tests are passing on my arm archlinux labtop: