Skip to content

Musicxml: fix testInferredTechnique test#33617

Open
RomanPudashkin wants to merge 1 commit into
musescore:mainfrom
RomanPudashkin:musicxml_fix_getPlayingTechnique
Open

Musicxml: fix testInferredTechnique test#33617
RomanPudashkin wants to merge 1 commit into
musescore:mainfrom
RomanPudashkin:musicxml_fix_getPlayingTechnique

Conversation

@RomanPudashkin
Copy link
Copy Markdown
Contributor

Resolves: #30434 (comment) (last failing test on Windows)

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 28, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 12a9a1e8-b536-4602-8f77-87dc114e8b4e

📥 Commits

Reviewing files that changed from the base of the PR and between 5f37d1a and 30d897e.

📒 Files selected for processing (3)
  • muse
  • src/importexport/musicxml/internal/import/importmusicxmlpass2.cpp
  • src/importexport/musicxml/tests/data/testInferredTechnique_ref.mscx
✅ Files skipped from review due to trivial changes (2)
  • muse
  • src/importexport/musicxml/tests/data/testInferredTechnique_ref.mscx

📝 Walkthrough

Walkthrough

The MusicXML import's getPlayingTechnique() now normalizes m_wordsText (plain-text conversion, simplified(), lowercase, strip ". , ; :") and matches techniques by iterating an ordered vector of phrase→PlayingTechniqueType pairs, returning the first substring match. The phrase list was expanded (including palm mute mapped to Undefined). A test reference PlayTechAnnotation was changed from playTechType="mute" to "open". The muse submodule pointer was updated.

🚥 Pre-merge checks | ✅ 1 | ❌ 4

❌ Failed checks (1 warning, 3 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The description provides a link to the issue being resolved but lacks detail about the actual changes made and implementation approach. Add more context about what the fix entails, how the getPlayingTechnique() method was modified, and why these changes resolve the test failure.
Linked Issues check ❓ Inconclusive The linked issue #30434 addresses testsuite failures with import/export decimal precision and other test failures. This PR specifically targets the testInferredTechnique test failure mentioned in a comment on that issue. Verify that the changes to getPlayingTechnique() and the test reference file actually resolve the identified test failure mentioned in issue #30434's comment.
Out of Scope Changes check ❓ Inconclusive Changes appear focused on fixing the testInferredTechnique test by updating the playing technique inference logic and test reference file, with a submodule update also included. Clarify whether the muse submodule update is necessary for this fix or if it's an unrelated change that should be in a separate commit.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: fixing the testInferredTechnique test in MusicXML import functionality.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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 and usage tips.

coderabbitai[bot]

This comment was marked as resolved.

@RomanPudashkin RomanPudashkin force-pushed the musicxml_fix_getPlayingTechnique branch from d33e6ff to 1880280 Compare May 28, 2026 15:09
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 28, 2026

Actionable comments posted: 0

@RomanPudashkin RomanPudashkin force-pushed the musicxml_fix_getPlayingTechnique branch from 1880280 to 5f37d1a Compare May 29, 2026 11:59
@RomanPudashkin RomanPudashkin force-pushed the musicxml_fix_getPlayingTechnique branch from 5f37d1a to 30d897e Compare May 29, 2026 12:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Testsuite : failures with import/export decimals (and others)

2 participants