search

musescore / MuseScore

MuseScore is an open source and free music notation software. For support, contribution, bug reports, visit MuseScore.org. Fork and make pull requests!
https://musescore.org
Other
12.14k stars 2.63k forks source link

Unit tests (in GitHub CI) don't seem to find chords_std.xml #22500

Closed Jojo-Schmitz closed 5 months ago

Jojo-Schmitz commented 5 months ago

Issue type

Other type of issue

Bug description

9: 09:59:45.456 | ERROR | main_thread | ChordList::read | Cannot open chord description: /home/runner/work/MuseScore/MuseScore/build.install/share/mscore-4.4//styles/chords_std.xml 9: 09:59:45.456 | ERROR | main_thread | ChordList::read | Cannot open chord description: /home/runner/work/MuseScore/MuseScore/build.install/share/mscore-4.4//styles/chords_std.xml

Steps to reproduce

Check any unit test output from GitHub CI

Screenshots/Screen recordings

image

MuseScore Version

all

Regression

Regression vs. 3.x

Operating system

Linux (which is what the unit tests use)

Additional context

This does clutter (some 1700 lines!) the unit test log (espt. its red color!) with lots of otherwise unneccessary error messages, making it easy to not see the tree in the forrest

Notice also the double / in the output, that'd be an easy fix though:

diff --git a/src/engraving/dom/chordlist.cpp b/src/engraving/dom/chordlist.cpp
index 49dcbe6795..8ddcd824d9 100644
--- a/src/engraving/dom/chordlist.cpp
+++ b/src/engraving/dom/chordlist.cpp
@@ -1821,12 +1821,12 @@ bool ChordList::read(const String& name)
     if (ftest.isAbsolute()) {
         path = name;
     } else {
-        path = configuration()->appDataPath() + "/styles/" + name;
+        path = configuration()->appDataPath() + "styles/" + name;
     }

     // default to chords_std.xml
     if (!FileInfo::exists(path)) {
-        path = configuration()->appDataPath() + "/styles/chords_std.xml";
+        path = configuration()->appDataPath() + "styles/chords_std.xml";
     }

     if (name.isEmpty()) {

See #22501

Jojo-Schmitz commented 5 months ago

Maybe I see what's going on there: it seems the tests are run off of the build.debug directory, not off of build.install

2024-04-20T11:05:08.4443705Z Setup MuseScore build environment
2024-04-20T11:05:08.4552534Z UpdateCTestConfiguration  from :/home/runner/work/MuseScore/MuseScore/build.debug/DartConfiguration.tcl
2024-04-20T11:05:08.4588222Z UpdateCTestConfiguration  from :/home/runner/work/MuseScore/MuseScore/build.debug/DartConfiguration.tcl
2024-04-20T11:05:08.4593298Z Test project /home/runner/work/MuseScore/MuseScore/build.debug
2024-04-20T11:05:08.4598060Z Constructing a list of tests
2024-04-20T11:05:08.4743711Z Done constructing a list of tests
2024-04-20T11:05:08.4744198Z Updating test list for fixtures
2024-04-20T11:05:08.4746947Z Added 0 tests to meet fixture requirements
2024-04-20T11:05:08.4749473Z Checking test dependency graph...
2024-04-20T11:05:08.4750126Z Checking test dependency graph end
2024-04-20T11:05:08.4752608Z test 1
2024-04-20T11:05:08.4754961Z       Start  1: muse_global_tests
2024-04-20T11:05:08.4755246Z 
2024-04-20T11:05:08.4755718Z 1: Test command: /home/runner/work/MuseScore/MuseScore/build.debug/src/framework/global/tests/muse_global_tests
2024-04-20T11:05:08.4758513Z 1: Test timeout computed to be: 10000000`

And it seems that the install step has just been omitted? In that case I'm quite puzzled though that a missing chords_std.xml seems to be the only issue?!?

cbjeukendrup commented 5 months ago

And it seems that the install step has just been omitted?

I thought the same. Let's see if #22503 solves it.

Jojo-Schmitz commented 5 months ago

It does es indeed. And reveals two other so far undetected errors, related to chord symbols and their import from Mu1.

Jojo-Schmitz commented 5 months ago

See #22508 for a fix for these failing unit tests

Jojo-Schmitz commented 5 months ago

Next issue: Are these errors supposed to happen?

1: [ RUN      ] Global_IO_IODeviceTests.Open_ReadOnly
1: 11:03:02.942 | INFO  | main_thread     | GlobalModule::onPreInit | log path: /root/.local/share/muse_global_tests/logs/MuseScore_240421_110302.log
1: 11:03:02.942 | INFO  | main_thread     | GlobalModule::onPreInit | === Started MuseScore Studio 4.4.0, build: 241121040 ===
1: 11:03:02.943 | ERROR | main_thread     | IODevice::write | "ASSERT FAILED!": isOpenModeWriteable(), file: /home/runner/work/MuseScore/MuseScore/src/framework/global/io/iodevice.cpp, line: 200
1: [       OK ] Global_IO_IODeviceTests.Open_ReadOnly (220 ms)
1: [ RUN      ] Global_IO_IODeviceTests.Open_WriteOnly
1: 11:03:03.163 | INFO  | main_thread     | GlobalModule::onPreInit | log path: /root/.local/share/muse_global_tests/logs/MuseScore_240421_1[103](https://github.com/musescore/MuseScore/actions/runs/8772061759/job/24070551147#step:10:104)03.log
1: 11:03:03.163 | INFO  | main_thread     | GlobalModule::onPreInit | === Started MuseScore Studio 4.4.0, build: 24112[104](https://github.com/musescore/MuseScore/actions/runs/8772061759/job/24070551147#step:10:105)0 ===
1: 11:03:03.164 | ERROR | main_thread     | IODevice::read  | "ASSERT FAILED!": isOpenModeReadable(), file: /home/runner/work/MuseScore/MuseScore/src/framework/global/io/iodevice.cpp, line: 152
1: 11:03:03.316 | INFO  | main_thread     | GlobalModule::onPreInit | log path: /root/.local/share/muse_global_tests/logs/MuseScore_240421_110303.log
1: 11:03:03.316 | INFO  | main_thread     | GlobalModule::onPreInit | === Started MuseScore Studio 4.4.0, build: 241121040 ===
1: 11:03:03.317 | ERROR | main_thread     | IODevice::read  | "ASSERT FAILED!": isOpenModeReadable(), file: /home/runner/work/MuseScore/MuseScore/src/framework/global/io/iodevice.cpp, line: 152
1: 11:03:03.469 | INFO  | main_thread     | GlobalModule::onPreInit | log path: /root/.local/share/muse_global_tests/logs/MuseScore_240421_110303.log
1: 11:03:03.469 | INFO  | main_thread     | GlobalModule::onPreInit | === Started MuseScore Studio 4.4.0, build: 241121040 ===
1: 11:03:03.470 | ERROR | main_thread     | IODevice::read  | "ASSERT FAILED!": isOpenModeReadable(), file: /home/runner/work/MuseScore/MuseScore/src/framework/global/io/iodevice.cpp, line: 126

and 

: [----------] 1 test from Engraving_RemoveTests
9: [ RUN      ] Engraving_RemoveTests.removeStaff
9: 11:04:19.226 | DEBUG | main_thread     | Score::endCmd   | Undo stack current macro child count: 104
9: 11:04:19.226 | ERROR | main_thread     | staffHasElements | HairPin is in staff 1
9: [       OK ] Engraving_RemoveTests.removeStaff (200 ms)
9: [----------] 1 test from Engraving_RemoveTests (200 ms total)

As far as I can tell the only other 'red flags' in the unit test output, regardless the tests succeed ?!?

The latter is an easy fix, just change LOGE() to LOGD(), consistent with other parts of the code nearby.

Jojo-Schmitz commented 5 months ago

And what about these warnings?

9: 12:33:30.929 | WARN  | main_thread     | XmlStreamReader::tryParseEntity | unknown ENTITY: ENTITY display_major "m:0:0 ma m:0:0"
9: 12:33:30.929 | WARN  | main_thread     | XmlStreamReader::tryParseEntity | unknown ENTITY: ENTITY display_amajor "m:0:0 &display_major; m:0:0"
9: 12:33:30.929 | WARN  | main_thread     | XmlStreamReader::tryParseEntity | unknown ENTITY: ENTITY display_minor "m:0:0 mi m:0:0"
9: 12:33:30.929 | WARN  | main_thread     | XmlStreamReader::tryParseEntity | unknown ENTITY: ENTITY display_diminished "m:0:0 degree m:0:0"
9: 12:33:30.929 | WARN  | main_thread     | XmlStreamReader::tryParseEntity | unknown ENTITY: ENTITY display_half_diminished "m:0:0 &display_minor; &seven; &d_alp; &alter_flat; &sfive; &d_arp; m:0:0"
9: 12:33:30.929 | WARN  | main_thread     | XmlStreamReader::tryParseEntity | unknown ENTITY: ENTITY display_suspended4_pre "m:0:0 sus m:0:0"
9: 12:33:30.930 | WARN  | main_thread     | XmlStreamReader::tryParseEntity | unknown ENTITY: ENTITY display_suspended "m:0:0 sus m:0:0"
9: 12:33:30.930 | WARN  | main_thread     | XmlStreamReader::tryParseEntity | unknown ENTITY: ENTITY display_sixnine "m:1:-2 s6 m:-4:7 s9 m:0:-5"
9: 12:33:30.930 | WARN  | main_thread     | XmlStreamReader::tryParseEntity | unknown ENTITY: ENTITY d_lp "m:1:1 ( m:0:-1"
9: 12:33:30.930 | WARN  | main_thread     | XmlStreamReader::tryParseEntity | unknown ENTITY: ENTITY d_rp "m:0:1 ) m:0:-1"
9: 12:33:30.930 | WARN  | main_thread     | XmlStreamReader::tryParseEntity | unknown ENTITY: ENTITY two "m:0:2 2 m:0:-2"
9: 12:33:30.930 | WARN  | main_thread     | XmlStreamReader::tryParseEntity | unknown ENTITY: ENTITY four "m:0:2 4 m:0:-2"
9: 12:33:30.930 | WARN  | main_thread     | XmlStreamReader::tryParseEntity | unknown ENTITY: ENTITY five "m:0:2 5 m:0:-2"
9: 12:33:30.930 | WARN  | main_thread     | XmlStreamReader::tryParseEntity | unknown ENTITY: ENTITY six "m:0:2 6 m:0:-2"
9: 12:33:30.930 | WARN  | main_thread     | XmlStreamReader::tryParseEntity | unknown ENTITY: ENTITY seven "m:0:2 7 m:0:-2"
9: 12:33:30.930 | WARN  | main_thread     | XmlStreamReader::tryParseEntity | unknown ENTITY: ENTITY nine "m:0:2 9 m:0:-2"
9: 12:33:30.930 | WARN  | main_thread     | XmlStreamReader::tryParseEntity | unknown ENTITY: ENTITY eleven "m:0:2 11 m:0:-2"
9: 12:33:30.930 | WARN  | main_thread     | XmlStreamReader::tryParseEntity | unknown ENTITY: ENTITY thirteen "m:0:2 13 m:0:-2"
9: 12:33:30.930 | WARN  | main_thread     | XmlStreamReader::tryParseEntity | unknown ENTITY: ENTITY stwo "m:0:-1 s2 m:0:1"
9: 12:33:30.930 | WARN  | main_thread     | XmlStreamReader::tryParseEntity | unknown ENTITY: ENTITY sfour "m:0:-1 s4 m:0:1"
9: 12:33:30.931 | WARN  | main_thread     | XmlStreamReader::tryParseEntity | unknown ENTITY: ENTITY sfive "m:0:-1 s5 m:0:1"
9: 12:33:30.931 | WARN  | main_thread     | XmlStreamReader::tryParseEntity | unknown ENTITY: ENTITY sseven "m:0:-1 s7 m:0:1"
9: 12:33:30.931 | WARN  | main_thread     | XmlStreamReader::tryParseEntity | unknown ENTITY: ENTITY snine "m:0:-1 s9 m:0:1"
9: 12:33:30.931 | WARN  | main_thread     | XmlStreamReader::tryParseEntity | unknown ENTITY: ENTITY seleven "m:0:-1 s11 m:0:1"
9: 12:33:30.931 | WARN  | main_thread     | XmlStreamReader::tryParseEntity | unknown ENTITY: ENTITY sthirteen "m:0:-1 s13 m:0:1"
9: 12:33:30.931 | WARN  | main_thread     | XmlStreamReader::tryParseEntity | unknown ENTITY: ENTITY atwo "m:0:3 s2 m:0:-3"
9: 12:33:30.931 | WARN  | main_thread     | XmlStreamReader::tryParseEntity | unknown ENTITY: ENTITY athree "m:0:3 s3 m:0:-3"
9: 12:33:30.931 | WARN  | main_thread     | XmlStreamReader::tryParseEntity | unknown ENTITY: ENTITY afour "m:0:3 s4 m:0:-3"
9: 12:33:30.931 | WARN  | main_thread     | XmlStreamReader::tryParseEntity | unknown ENTITY: ENTITY afive "m:0:3 s5 m:0:-3"
9: 12:33:30.931 | WARN  | main_thread     | XmlStreamReader::tryParseEntity | unknown ENTITY: ENTITY aseven "m:0:3 s7 m:0:-3"
9: 12:33:30.931 | WARN  | main_thread     | XmlStreamReader::tryParseEntity | unknown ENTITY: ENTITY anine "m:0:3 s9 m:0:-3"
9: 12:33:30.931 | WARN  | main_thread     | XmlStreamReader::tryParseEntity | unknown ENTITY: ENTITY alter_flat "m:0:0 sb m:0:0"
9: 12:33:30.931 | WARN  | main_thread     | XmlStreamReader::tryParseEntity | unknown ENTITY: ENTITY alter_sharp "m:0:0 s# m:0:0"
9: 12:33:30.931 | WARN  | main_thread     | XmlStreamReader::tryParseEntity | unknown ENTITY: ENTITY alter_major "m:1:-5 &display_amajor; m:0:5"
9: 12:33:30.931 | WARN  | main_thread     | XmlStreamReader::tryParseEntity | unknown ENTITY: ENTITY display_augmented "m:0:0 a u g m:0:0"
9: 12:33:30.932 | WARN  | main_thread     | XmlStreamReader::tryParseEntity | unknown ENTITY: ENTITY plus "m:0:0 + m:0:0"
9: 12:33:30.932 | WARN  | main_thread     | XmlStreamReader::tryParseEntity | unknown ENTITY: ENTITY add "m:0:2 add m:0:-2"
9: 12:33:30.932 | WARN  | main_thread     | XmlStreamReader::tryParseEntity | unknown ENTITY: ENTITY alt "m:0:-5 a l t m:0:5"
9: 12:33:30.932 | WARN  | main_thread     | XmlStreamReader::tryParseEntity | unknown ENTITY: ENTITY omit "m:1:0 o m i t m:0:0"
9: 12:33:30.932 | WARN  | main_thread     | XmlStreamReader::tryParseEntity | unknown ENTITY: ENTITY no "m:0:0 n o m:0:0"
9: 12:33:30.932 | WARN  | main_thread     | XmlStreamReader::tryParseEntity | unknown ENTITY: ENTITY blues "m:0:0 letterb l u e s m:0:0"
9: 12:33:30.932 | WARN  | main_thread     | XmlStreamReader::tryParseEntity | unknown ENTITY: ENTITY Tristan "m:0:0 t r i s t a n m:0:0"
9: 12:33:30.932 | WARN  | main_thread     | XmlStreamReader::tryParseEntity | unknown ENTITY: ENTITY Lyd "m:0:0 l y d m:0:0"
9: 12:33:30.932 | WARN  | main_thread     | XmlStreamReader::tryParseEntity | unknown ENTITY: ENTITY Phryg "m:0:0 p h r y g m:0:0"

@MarcSabatella might know more about those, seems to be the old-style (?) formatting of chord symbols. Looks like tryParseEntity() can't split those strings into 3 substrings separated by spaces, due to spaces inside the double quotes. Looks like a real bug to me.

cbjeukendrup commented 5 months ago

1: 11:03:02.943 | ERROR | main_thread | IODevice::write | "ASSERT FAILED!": isOpenModeWriteable(), file: /home/runner/work/MuseScore/MuseScore/src/framework/global/io/iodevice.cpp, line: 200

Yes, these are expected. Those tests are basically verifying that this assertion failure occurs. Bit weird perhaps, but at least intentional...

9: 11:04:19.226 | ERROR | main_thread | staffHasElements | HairPin is in staff 1

Yes, changing that to LOGD() makes sense to me.

MarcSabatella commented 5 months ago

I guess those are from tests created in MuseScore 1? Those definitions come from the old “cchords” chord description files in share/styles. I don’t think it was ever intended that the entity be parsed into separate substrings - the string was meant to be taken in whole, spaces intact. As far as I know spaces are legal within entity declarations, as are references to other entities. Is there something not actually working, or are these just spurious warnings?

Jojo-Schmitz commented 5 months ago

Happens in a unit test 'importing' a Mu1 score, (after having fixed the issue at hand here)

Actually when reading ...\src\engraving\tests\compat114_data\textstyles.mscx, and that indeed uses cchords_muse.xml as its chord description file

Jojo-Schmitz commented 5 months ago

The regular (minmal) syntax for them is

<!ENTITY entity-name "entity-value">

The full syntax is:

<!ENTITY [%] Name [SYSTEM|PUBLIC] "Value" [additional info] >

So it seems our parser, simply cutting it into 3 space separated parts, isn't good enough, and that completly independant from what these chord descriptions files do, they are just the victim here.

Probably worth another issue...

cbjeukendrup commented 5 months ago

Probably worth another issue...

Yes, I think that would be good

Jojo-Schmitz commented 5 months ago

Well, I've just fixed it in the passing ;-) Please review...

The fact that it doesn't show further unit test fails is probably due to those ENTITY things just influencing the rendering of chords, so it would only show in the vtests, apparently there are none testing Mu1 chord symbols (and I don't really believe there should be any)

cbjeukendrup commented 5 months ago

Actually we should have unit tests for the XmlStreamReader class. Not sure though whether that belongs in the scope of this issue.

Jojo-Schmitz commented 5 months ago

Yeah, but no vtests for Mu1 chord symbols. Any unit tests for that class seem to be beyond my pay-grade :-(

Jojo-Schmitz commented 3 weeks ago

There are still/again quite a lot of these, all in the engraving tests:

10: 13:46:52.504 | ERROR | main_thread | ChordList::read | Cannot open chord description: /styles/chords_std.xml

Apparently the appDataPath parameter to bool ChordList::read(const muse::io::path_t& appDataPath, const String& name) is empty