Skip to content

Fix failing MusicXml_Tests.glissandoStyle: update its reference for the new order of styled line properties - #35223

Merged
miiizen merged 1 commit into
musescore:mainfrom
ragesoss:fix-glissando-style-test-ref
Oct 14, 2026
Merged

miiizen merged 1 commit into
musescore:mainfrom
ragesoss:fix-glissando-style-test-ref

Conversation

@ragesoss

@ragesoss ragesoss commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

CI is failing because of the test added by my recent commit, and another change that got merged meanwhile. I had Claude Opus 5.5 do a quick fix. AI writeup below.

-ragesoss


No issue: MusicXml_Tests.glissandoStyle fails on main (for example in the unit-test run of #35219: https://github.com/musescore/MuseScore/actions/runs/38433065427).

#35186 added this test with a reference file saved before #35118 (Refactor SLine styled properties) was merged. #35118 changed the order in which a line's styled properties are written, so a glissando's <glissandoStyle> is now saved after <diagonal> instead of right after <text>. Each PR passed its checks on its own; they were merged about an hour apart. This moves that one line in testGlissandoStyle_ref.mscx; nothing else changes.

Checked with iex_musicxml_tests built from main (d61794c): glissandoStyle fails there, and with this reference all 364 MusicXML tests pass.

  • I signed the CLA.
  • The title of the PR describes the problem it addresses.
  • The code in the PR follows the coding rules.
  • There are no unnecessary changes.
  • Each commit's message describes its purpose and effects, and references the issue it resolves. If changes are extensive, there is a sequence of easily reviewable commits.
  • The code compiles and runs on my machine, preferably after each commit individually. I have manually tested and verified that my changes fulfil their intended purpose.
  • No prior attempts to resolve this problem exist, or if they do, I listed them in my PR description and described how I avoided repeating past mistakes.

Complete only if applicable:

  • I created a unit test or vtest to verify the changes I made.
  • This PR was created using AI assistance; I have read, understood and complied following the guidelines for AI-assisted contributions. The AI tool(s) I used are listed below.
  • Claude Code (Claude Opus 5.5). It prepared this PR, including the commit message and this description: it found the failure while working on another MusicXML change, traced it to the two merges, and checked the fix as described above.

🤖 Generated with Claude Code

…y order

musescore#35118 changed the order in which a line's styled properties are written,
and musescore#35186 added this reference with the old order; each passed on its own,
and since both were merged MusicXml_Tests.glissandoStyle fails. Only the
place of <glissandoStyle> in the glissando changes.

Assisted-by: Claude Opus 5.5
@ragesoss
ragesoss marked this pull request as ready for review October 13, 2026 18:54
@coderabbitai

coderabbitai Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: musescore/MuseScore/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: e0360251-eb22-4c85-a8b0-a52ea43f7d0c
📥 Commits

Reviewing files that changed from the base of the PR and between d61794c and 57ff5b6.

📒 Files selected for processing: 1
  • src/importexport/musicxml/tests/data/testGlissandoStyle_ref.mscx

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

The MusicXML test fixture moves the glissandoStyle element after the glissando’s other properties. Its value remains portamento, and the other property values are unchanged.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 57ff5

This change only updates the expected XML ordering in one test fixture; no actionable merge-blocking risk is evident.

Pre-merge checks: Passed 5
Passed Passed checks: 5 passed
Check name Status Explanation
Docstring Coverage Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Title check Passed The title clearly identifies the failing test and the required reference-file update caused by the new styled-property order.
Description check Passed The description explains the failure, identifies the cause, describes the one-line fix, reports test verification, and completes the applicable checklist items. It also documents the AI assistance use…
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Linked repositories: Public OSS repositories can only analyze public repositories installed in this organization. No linked repositories were analyzed; skipped musescore/muse_framework.git.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@miiizen
miiizen merged commit 0ec7065 into musescore:main Oct 14, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants