Skip to content

Commit 1a9ffd6

Browse files
authored
Merge pull request #3 from crespinl/webmscore
Add a real test suite, and fix the bugs it found
2 parents 8ec6401 + 2a33a46 commit 1a9ffd6

30 files changed

Lines changed: 1251 additions & 43 deletions

‎.github/workflows/build.yml‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -143,16 +143,32 @@ jobs:
143143
with:
144144
name: npm-pack
145145
path: web-example
146-
- name: Test
147-
id: run-test
146+
- name: Install
148147
run: |
149148
cd web-example
150149
npm i
151150
npm i ./webmscore4-*.tgz
151+
- name: Test
152+
run: |
153+
cd web-example
154+
npm run test:ci
155+
- name: Smoke test
156+
# also produces the exported files uploaded below
157+
id: run-test
158+
if: ${{ !cancelled() }}
159+
run: |
160+
cd web-example
152161
npm start || echo "failed=1" >> "$GITHUB_OUTPUT"
153162
npm run test-synth || echo "failed=1" >> "$GITHUB_OUTPUT"
154163
155164
- uses: actions/upload-artifact@v7
165+
if: ${{ !cancelled() }}
166+
with:
167+
name: test-results
168+
path: web-example/junit.xml
169+
170+
- uses: actions/upload-artifact@v7
171+
if: ${{ !cancelled() }}
156172
with:
157173
name: example-exports
158174
path: web-example/exported*

‎CLAUDE.md‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -112,12 +112,24 @@ Smoke test (needs Node; the JS shims assume a browser-ish environment):
112112
cp web/build.release/webmscore.lib.* web-public/
113113
cd web-public && npm run build:js-only
114114
cd ../web-example && rm -rf node_modules/webmscore4 && npm i ../web-public --install-links
115-
npm start # export MusicXML/SVG/PNG/PDF/MXL/MIDI/OGG/positions/metadata
116-
npm run test-synth # audio engine: synthesis + per-track output params
115+
npm test # the test suite (node:test, web-example/test/*.test.js), ~1 min
116+
npm start # smoke test: export MusicXML/SVG/PNG/PDF/MXL/MIDI/OGG/positions/metadata
117+
npm run test-synth # smoke test: synthesis + per-track output params
117118
```
118119

119-
`npm start` covers rendering and file export; `test-synth` is the one that exercises the
120-
audio engine and the RPC plumbing — always run both.
120+
`npm test` is the check that matters after a rebase; the two smoke tests only fail on a crash
121+
(CI still runs them, for the exported files it uploads). Reinstall the package
122+
(`rm -rf node_modules/webmscore4 && npm i ...`) after each rebuild: a plain `npm i` keeps the
123+
old copy, since the version did not change.
124+
125+
Writing tests (`web-example/test/helpers.js` has the shared loaders/parsers):
126+
127+
* each test file runs in its own process (= its own wasm instance), one file at a time;
128+
state that leaks between tests (soundfont, current audio sequence) only leaks within a file
129+
* assert invariants (page counts, magic bytes, RMS, contiguity of measures…), not exact bytes,
130+
so that tests survive upstream rebases
131+
* known bugs are `{ todo: '…' }` tests: reported, but they do not fail the run — when one
132+
starts passing, drop the `todo`
121133

122134
## webmscore-specific switches
123135

@@ -144,6 +156,13 @@ audio engine and the RPC plumbing — always run both.
144156
`web/build.release/webmscore.lib.js.symbols`: `grep -m1 "^<index>:" …lib.js.symbols`.
145157
`null function or function signature mismatch` almost always means a null pointer was
146158
dereferenced through a virtual call (usually a service webmscore does not register).
159+
* **The playback follows the *current* score.** The audio engine has a single sequence, the one
160+
of the global context's current notation. `makeCurrent()` (`web/main.cpp`) must run before any
161+
audio operation on a score; switching rebuilds the sequence from the project's audio settings,
162+
so params must be stored there (`web/audio/audioparams.cpp`), not only in the engine.
163+
* **rpc in single-thread mode**: main and engine share one dispatch table in
164+
`GeneralRpcChannel`, so a message reaches the handlers of *both* sides. Responses must only be
165+
matched by call id; a handler registered for a method must not assume which side sent it.
147166
* **No threads, no event loop.** Anything asynchronous (promises, `async::processMessages`,
148167
the audio RPC queue) only advances when webmscore pumps it explicitly — see
149168
`Synth::getSequenceId()` in `web/audio/audiosynth.cpp` and the wait loops in

‎README.md‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,19 @@ npm start # Node.js example
198198
npm run start:browser # browser example
199199
```
200200

201+
## Tests
202+
203+
The test suite (Node.js ≥ 22) runs against the package installed in `web-example`:
204+
205+
```sh
206+
cd ./web-example
207+
npm i ../web-public --install-links
208+
npm test
209+
```
210+
211+
Tests live in [web-example/test](./web-example/test), one file per area (loading, exports, positions,
212+
metadata, audio). Known bugs are kept as `todo` tests: they are reported but do not fail the run.
213+
201214
## Debugging
202215

203216
See [How to look up function names in the .symbols file?](https://github.com/LibreScore/webmscore/blob/web/CHANGELOG.md#0192---2021-01-25)

‎src/framework/audio/common/rpc/platform/general/generalrpcchannel.cpp‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,10 @@ void GeneralRpcChannel::receive(RpcData& to, const Msg& m) const
135135
}
136136

137137
// by method
138-
{
138+
//! NOTE Not for responses: they carry the method of their request. In single-thread mode
139+
//! (webmscore) both sides share one dispatch table, so the request handler would get its own
140+
//! response, handle it as a new request and respond again, forever
141+
if (m.type != MsgType::Response) {
139142
auto it = to.onMethods.find(m.method);
140143
if (it != to.onMethods.end() && it->second) {
141144
it->second(m);

‎src/framework/audio/main/internal/player.cpp‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,11 @@ void Player::init()
4848
});
4949

5050
Msg msg = rpc::make_request(Method::GetPlaybackStatus, RpcPacker::pack(m_sequenceId));
51-
channel()->send(msg, [this](const Msg& res) {
51+
channel()->send(msg, [this, alive = std::weak_ptr<bool>(m_alive)](const Msg& res) {
5252
ONLY_AUDIO_MAIN_THREAD;
53+
if (alive.expired()) {
54+
return;
55+
}
5356
PlaybackStatus status = PlaybackStatus::Stopped;
5457
StreamId streamId = 0;
5558
IF_ASSERT_FAILED(RpcPacker::unpack(res.data, status, streamId)) {
@@ -68,8 +71,11 @@ void Player::init()
6871
});
6972

7073
Msg msg = rpc::make_request(Method::GetPlaybackPosition, RpcPacker::pack(m_sequenceId));
71-
channel()->send(msg, [this](const Msg& res) {
74+
channel()->send(msg, [this, alive = std::weak_ptr<bool>(m_alive)](const Msg& res) {
7275
ONLY_AUDIO_MAIN_THREAD;
76+
if (alive.expired()) {
77+
return;
78+
}
7379
secs_t pos = 0.0;
7480
StreamId streamId = 0;
7581
IF_ASSERT_FAILED(RpcPacker::unpack(res.data, pos, streamId)) {

‎src/framework/audio/main/internal/player.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
#ifndef MUSE_AUDIO_PLAYER_H
2424
#define MUSE_AUDIO_PLAYER_H
2525

26+
#include <memory>
27+
2628
#include "../iplayer.h"
2729

2830
#include "global/async/asyncable.h"
@@ -63,6 +65,10 @@ class Player : public IPlayer, public async::Asyncable, public Contextable
6365
private:
6466

6567
TrackSequenceId m_sequenceId = -1;
68+
69+
//! NOTE The responses to the requests sent in `init()` can arrive after the player is destroyed
70+
//! (e.g. when the current notation changes before the rpc channel is processed)
71+
std::shared_ptr<bool> m_alive = std::make_shared<bool>(true);
6672
PlaybackStatus m_playbackStatus = PlaybackStatus::Stopped;
6773
async::Channel<PlaybackStatus> m_playbackStatusChanged;
6874

‎src/playback/internal/playbackcontroller.cpp‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1717,17 +1717,17 @@ void PlaybackController::setNotation(notation::INotationPtr notation)
17171717
return;
17181718
}
17191719

1720-
#ifndef __EMSCRIPTEN__
17211720
if (m_notation) {
1721+
#ifndef __EMSCRIPTEN__
17221722
INotationPartsPtr notationParts = m_notation->parts();
17231723
NotifyList<const Part*> partList = notationParts->partList();
17241724
partList.disconnect(this);
17251725

17261726
m_notation->interaction()->selectionChanged().disconnect(this);
17271727
m_notation->interaction()->textEditingEnded().disconnect(this);
1728+
#endif
17281729
m_notation->soloMuteState()->trackSoloMuteStateChanged().disconnect(this);
17291730
}
1730-
#endif
17311731

17321732
m_notation = notation;
17331733

@@ -1767,12 +1767,13 @@ void PlaybackController::setNotation(notation::INotationPtr notation)
17671767
playElements({ text });
17681768
}
17691769
});
1770+
#endif
17701771

1772+
// webmscore: the solo/mute states are set through `setTrackSoloMuteState()` (audio output params)
17711773
m_notation->soloMuteState()->trackSoloMuteStateChanged().onReceive(
17721774
this, [this](const InstrumentTrackId&, const notation::INotationSoloMuteState::SoloMuteState&) {
17731775
updateSoloMuteStates();
17741776
});
1775-
#endif
17761777
}
17771778

17781779
void PlaybackController::setMasterNotation(notation::IMasterNotationPtr masterNotation)

‎web-example/.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,4 @@ node_modules
22
package-lock.json
33
exported*
44
benchmark/
5+
junit.xml

‎web-example/package.json‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
"webmscore": "../web-public"
99
},
1010
"scripts": {
11+
"test": "node --test --test-concurrency=1 \"test/*.test.js\"",
12+
"test:ci": "node --test --test-concurrency=1 --test-reporter=spec --test-reporter-destination=stdout --test-reporter=junit --test-reporter-destination=junit.xml \"test/*.test.js\"",
1113
"start": "node --experimental-modules --unhandled-rejections=strict example.js",
1214
"test-synth": "node --experimental-modules --unhandled-rejections=strict test_synth.js",
1315
"start:browser": "npx http-server . -o -c-1",
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
import { describe, it, before } from 'node:test'
2+
import assert from 'node:assert/strict'
3+
import { loadScore, setSoundFont, hasMagic, parseWav, rms } from './helpers.js'
4+
5+
// a short score: rendering is about 2.5x real time
6+
const SCORE = 'repeat01.mscx'
7+
8+
const MAGIC = {
9+
wav: (d) => hasMagic(d, 'RIFF') && hasMagic(d, 'WAVE', 8),
10+
ogg: (d) => hasMagic(d, 'OggS'),
11+
flac: (d) => hasMagic(d, 'fLaC'),
12+
mp3: (d) => hasMagic(d, 'ID3') || (d[0] === 0xff && (d[1] & 0xe0) === 0xe0),
13+
}
14+
15+
describe('saveAudio', () => {
16+
let score, duration
17+
before(async () => {
18+
score = await loadScore(SCORE)
19+
duration = (await score.metadata()).duration
20+
})
21+
22+
it('throws without a soundfont', async () => {
23+
await assert.rejects(score.saveAudio('ogg'), /soundfont/)
24+
})
25+
26+
it('wav: the whole score, not silent', async () => {
27+
await setSoundFont()
28+
const wav = parseWav(await score.saveAudio('wav'))
29+
assert.equal(wav.channels, 2)
30+
assert.equal(wav.sampleRate, 44100)
31+
assert.ok(wav.duration >= duration && wav.duration < duration + 10, `${wav.duration}s vs ${duration}s`)
32+
assert.ok(rms(wav.samples) > 0.01, 'not silent')
33+
})
34+
35+
// each format is a new export of the same score (used to work only once per instance)
36+
for (const format of ['ogg', 'flac', 'mp3']) {
37+
it(`${format}: valid file of a plausible size`, async () => {
38+
const data = await score.saveAudio(format)
39+
assert.ok(MAGIC[format](data), 'magic bytes')
40+
// > 8 kbit/s: a silent or truncated file is a few KB at most
41+
assert.ok(data.length > duration * 1000, `${data.length} bytes`)
42+
})
43+
}
44+
45+
it('rejects an unknown format', async () => {
46+
await assert.rejects(score.saveAudio('aiff'))
47+
})
48+
49+
it('exports the same audio twice', async () => {
50+
const first = parseWav(await score.saveAudio('wav'))
51+
const second = parseWav(await score.saveAudio('wav'))
52+
assert.equal(second.samples.length, first.samples.length)
53+
assert.ok(Math.abs(rms(second.samples) - rms(first.samples)) < rms(first.samples) * 0.05)
54+
})
55+
56+
it('exports after synthAudio', async () => {
57+
const fn = await score.synthAudio(0)
58+
await fn()
59+
await fn(true)
60+
const wav = parseWav(await score.saveAudio('wav'))
61+
assert.ok(wav.duration >= duration, `${wav.duration}s vs ${duration}s`)
62+
})
63+
64+
it('exports its own score, not the last loaded one', async () => {
65+
const other = await loadScore('Aequale_No_1.mscz')
66+
const wav = parseWav(await score.saveAudio('wav'))
67+
assert.ok(wav.duration >= duration && wav.duration < duration + 10, `${wav.duration}s vs ${duration}s`)
68+
const { duration: otherDuration } = await other.metadata()
69+
assert.ok(otherDuration > duration + 10)
70+
})
71+
})

0 commit comments

Comments
 (0)