Skip to content

Commit 125d62c

Browse files
committed
Simplify replaceInstrument() to use DOM-level ChangePart directly
Remove dependency on notation module by using engraving::ChangePart command directly instead of going through INotationParts. This: - Eliminates need for NotationMock and NotationPartsMock in tests - Simplifies test setup (no mock injection required) - Keeps the implementation purely at the engraving layer - Makes the API self-contained within the plugin API module
1 parent 569a1e6 commit 125d62c

File tree

5 files changed

+25
-265
lines changed

5 files changed

+25
-265
lines changed

src/engraving/api/tests/mocks/globalcontextmock.h

Lines changed: 0 additions & 46 deletions
This file was deleted.

src/engraving/api/tests/mocks/notationmock.h

Lines changed: 0 additions & 65 deletions
This file was deleted.

src/engraving/api/tests/mocks/notationpartsmock.h

Lines changed: 0 additions & 85 deletions
This file was deleted.

src/engraving/api/tests/score_tests.cpp

Lines changed: 20 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -33,38 +33,11 @@
3333
#include "engraving/api/v1/score.h"
3434
#include "engraving/api/v1/part.h"
3535

36-
#include "mocks/globalcontextmock.h"
37-
#include "mocks/notationmock.h"
38-
#include "mocks/notationpartsmock.h"
39-
4036
using namespace mu::engraving;
41-
using namespace mu::notation;
42-
using namespace mu::context;
43-
using ::testing::Return;
44-
using ::testing::_;
45-
using ::testing::NiceMock;
4637

4738
class Engraving_ApiScoreTests : public ::testing::Test
4839
{
4940
public:
50-
void SetUp() override
51-
{
52-
m_globalContext = std::make_shared<NiceMock<GlobalContextMock> >();
53-
m_notation = std::make_shared<NiceMock<NotationMock> >();
54-
m_notationParts = std::make_shared<NiceMock<NotationPartsMock> >();
55-
}
56-
57-
void TearDown() override
58-
{
59-
m_notationParts.reset();
60-
m_notation.reset();
61-
m_globalContext.reset();
62-
}
63-
64-
protected:
65-
std::shared_ptr<NiceMock<GlobalContextMock> > m_globalContext;
66-
std::shared_ptr<NiceMock<NotationMock> > m_notation;
67-
std::shared_ptr<NiceMock<NotationPartsMock> > m_notationParts;
6841
};
6942

7043
//---------------------------------------------------------
@@ -197,14 +170,13 @@ TEST_F(Engraving_ApiScoreTests, replaceInstrumentRedo)
197170
}
198171

199172
//---------------------------------------------------------
200-
// testReplaceInstrumentApiWithMocks
173+
// testReplaceInstrumentApi
201174
// Test the Plugin API Score::replaceInstrument() method
202-
// using mocks to verify the correct call chain
203175
//---------------------------------------------------------
204176

205-
TEST_F(Engraving_ApiScoreTests, replaceInstrumentApiWithMocks)
177+
TEST_F(Engraving_ApiScoreTests, replaceInstrumentApi)
206178
{
207-
// [GIVEN] A score with a part and mocked notation layer
179+
// [GIVEN] A score with a part
208180
MasterScore* domScore = compat::ScoreAccess::createMasterScore(nullptr);
209181

210182
Part* domPart = new Part(domScore);
@@ -213,34 +185,22 @@ TEST_F(Engraving_ApiScoreTests, replaceInstrumentApiWithMocks)
213185

214186
// Set initial instrument
215187
Instrument initialInstrument;
216-
initialInstrument.setId(u"wind.flutes.flute");
188+
initialInstrument.setId(u"test.flute");
217189
initialInstrument.setTrackName(u"Flute");
218190
domPart->setInstrument(initialInstrument);
219191

220-
// Setup mock chain: GlobalContext -> Notation -> NotationParts
221-
ON_CALL(*m_globalContext, currentNotation())
222-
.WillByDefault(Return(m_notation));
223-
ON_CALL(*m_notation, parts())
224-
.WillByDefault(Return(m_notationParts));
225-
226-
// [EXPECT] replaceInstrument to be called on NotationParts with correct parameters
227-
EXPECT_CALL(*m_notationParts, replaceInstrument(_, _, _))
228-
.Times(1);
192+
EXPECT_EQ(domPart->instrumentId(), QString("test.flute"));
229193

230-
// Register the mock in IOC
231-
muse::modularity::globalIoc()->registerExport<IGlobalContext>("test", m_globalContext);
232-
233-
// Create API wrapper for the score
194+
// Create API wrappers
234195
apiv1::Score apiScore(domScore);
235-
236-
// Create API wrapper for the part
237196
apiv1::Part* apiPart = new apiv1::Part(domPart, apiv1::Ownership::SCORE);
238197

239-
// [WHEN] We call replaceInstrument through the API
198+
// [WHEN] We call replaceInstrument through the API with a valid instrument
240199
apiScore.replaceInstrument(apiPart, "violin");
241200

242-
// Cleanup
243-
muse::modularity::globalIoc()->unregister<IGlobalContext>("test");
201+
// [THEN] The instrument should be changed to violin
202+
EXPECT_EQ(domPart->instrumentId(), QString("violin"));
203+
244204
delete apiPart;
245205
delete domScore;
246206
}
@@ -257,7 +217,7 @@ TEST_F(Engraving_ApiScoreTests, replaceInstrumentApiNullPart)
257217
apiv1::Score apiScore(domScore);
258218

259219
// [WHEN/THEN] Calling with null part should not crash
260-
apiScore.replaceInstrument(nullptr, "strings.violin");
220+
apiScore.replaceInstrument(nullptr, "violin");
261221

262222
delete domScore;
263223
}
@@ -276,13 +236,20 @@ TEST_F(Engraving_ApiScoreTests, replaceInstrumentApiInvalidInstrument)
276236
domScore->appendPart(domPart);
277237
domScore->appendStaff(Factory::createStaff(domPart));
278238

239+
// Set initial instrument
240+
Instrument initialInstrument;
241+
initialInstrument.setId(u"test.initial");
242+
domPart->setInstrument(initialInstrument);
243+
279244
apiv1::Score apiScore(domScore);
280245
apiv1::Part* apiPart = new apiv1::Part(domPart, apiv1::Ownership::SCORE);
281246

282-
// [WHEN/THEN] Calling with invalid instrument ID should not crash
283-
// (instrument templates are not loaded in test environment, so any ID is "invalid")
247+
// [WHEN] Calling with invalid instrument ID
284248
apiScore.replaceInstrument(apiPart, "nonexistent.instrument.xyz");
285249

250+
// [THEN] Instrument should remain unchanged
251+
EXPECT_EQ(domPart->instrumentId(), QString("test.initial"));
252+
286253
delete apiPart;
287254
delete domScore;
288255
}

src/engraving/api/v1/score.cpp

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -30,14 +30,10 @@
3030
#include "dom/score.h"
3131
#include "dom/segment.h"
3232
#include "dom/text.h"
33+
#include "editing/editpart.h"
3334
#include "editing/editsystemlocks.h"
3435
#include "types/typesconv.h"
3536

36-
// notation
37-
#include "notation/inotation.h"
38-
#include "notation/inotationparts.h"
39-
#include "notation/notationtypes.h"
40-
4137
// api
4238
#include "apistructs.h"
4339
#include "cursor.h"
@@ -184,18 +180,11 @@ void Score::replaceInstrument(apiv1::Part* part, const QString& instrumentId)
184180
return;
185181
}
186182

187-
mu::notation::INotationPartsPtr parts = notation() ? notation()->parts() : nullptr;
188-
if (!parts) {
189-
LOGW("replaceInstrument: notation parts is null");
190-
return;
191-
}
192-
193-
mu::notation::InstrumentKey instrumentKey;
194-
instrumentKey.partId = muse::ID(part->part()->id());
195-
instrumentKey.instrumentId = muse::String::fromQString(part->part()->instrumentId());
196-
instrumentKey.tick = mu::engraving::Part::MAIN_INSTRUMENT_TICK;
183+
mu::engraving::Part* domPart = part->part();
184+
mu::engraving::Instrument newInstrument = mu::engraving::Instrument::fromTemplate(t);
185+
String newPartName = t->trackName;
197186

198-
parts->replaceInstrument(instrumentKey, mu::engraving::Instrument::fromTemplate(t));
187+
score()->undo(new ChangePart(domPart, new mu::engraving::Instrument(newInstrument), newPartName));
199188
}
200189

201190
//---------------------------------------------------------

0 commit comments

Comments
 (0)