Skip to content

Commit 4e1b4c0

Browse files
PascalThuetclaude
andauthored
fix(tests): correct unit tests that pass without verifying behavior (Dash-Industry-Forum#5012)
* fix(tests): correct unit tests that pass without verifying behavior - DashParser: replace .bind() with arrow functions so parse() is actually called with the intended arguments (lines 17-28) - DashParser: move async fixture loading from describe() scope into a before() hook — Mocha does not support async describe (line 64) - MssParser: same .bind() fix (line 62) - Stream: await the Promise from setMediaSource() so the assertion runs before Mocha marks the test as passed (lines 93-98) Fixes Dash-Industry-Forum#5011 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(parser): reject non-MPD XML input in DashParser.parse() parseXml() can return a non-null object for any parseable XML-like string. The existing guard only checked for null, so non-MPD input (e.g. plain text) would pass validation and crash later when accessing .protocol on undefined. Add a check for the presence of MPD or Patch root elements before proceeding. This gives callers the expected 'failed to parse the manifest' error instead of a cryptic property access error. Bug exposed by fixing the .bind() test pattern in Dash-Industry-Forum#5012. Refs Dash-Industry-Forum#5011 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test(dash): cover patch root parsing in DashParser --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent aac772a commit 4e1b4c0

4 files changed

Lines changed: 31 additions & 14 deletions

File tree

src/dash/parser/DashParser.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ function DashParser(config) {
109109

110110
manifest = parseXml(data);
111111

112-
if (!manifest) {
112+
if (!manifest || (!manifest.MPD && !manifest.Patch)) {
113113
throw new Error('failed to parse the manifest');
114114
}
115115

test/unit/test/dash/dash.DashParser.js

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,17 +15,33 @@ const dashManifestModel = DashManifestModel(context).getInstance();
1515
describe('DashParser', function () {
1616

1717
it('should throw an error when parse is called without data and config object has been set properly', () => {
18-
expect(dashParser.parse.bind('')).to.be.throw('failed to parse the manifest');
18+
expect(() => dashParser.parse('')).to.throw('failed to parse the manifest');
1919
});
2020

2121
it('should throw an error when parse is called with invalid data', async () => {
2222
let manifest = await FileLoader.loadTextFile('/data/dash/manifest_error.xml');
23-
expect(dashParser.parse.bind(manifest)).to.be.throw('failed to parse the manifest');
23+
expect(() => dashParser.parse(manifest)).to.throw('failed to parse the manifest');
2424
});
2525

2626
it('should return an Object when parse is called with correct data', async () => {
2727
let manifest = await FileLoader.loadTextFile('/data/dash/manifest.xml');
28-
expect(dashParser.parse.bind(manifest)).to.be.instanceOf(Object);
28+
expect(dashParser.parse(manifest)).to.be.instanceOf(Object);
29+
});
30+
31+
it('should return a parsed Patch object when parse is called with valid patch data', () => {
32+
const patchManifest = `<?xml version="1.0" encoding="UTF-8"?>
33+
<Patch mpdId="foobar"
34+
publishTime="2020-01-01T00:00:01Z"
35+
originalPublishTime="2020-01-01T00:00:00Z">
36+
<replace sel="/MPD/@publishTime">2020-01-01T00:00:01Z</replace>
37+
</Patch>`;
38+
const parsedPatch = dashParser.parse(patchManifest);
39+
40+
expect(parsedPatch).to.be.instanceOf(Object);
41+
expect(parsedPatch.protocol).to.equal('DASH');
42+
expect(parsedPatch.mpdId).to.equal('foobar');
43+
expect(parsedPatch.replace).to.be.instanceOf(Array);
44+
expect(parsedPatch.replace).to.have.lengthOf(1);
2945
});
3046

3147
describe('DashParser matchers', function () {
@@ -61,15 +77,19 @@ describe('DashParser', function () {
6177
});
6278
});
6379

64-
describe('DashParser - ObjectIron', async () => {
80+
describe('DashParser - ObjectIron', () => {
81+
let manifest_prop;
82+
83+
before(async () => {
84+
manifest_prop = await FileLoader.loadTextFile('/data/dash/manifest_properties.xml');
85+
});
86+
6587
beforeEach(function () {
6688
dashManifestModel.setConfig({
6789
errHandler: errorHandlerMock
6890
});
6991
});
7092

71-
let manifest_prop = await FileLoader.loadTextFile('/data/dash/manifest_properties.xml');
72-
7393
it('should map AudioChannelConfig even if another instance is present on Representation', async () => {
7494
let parsedMpd = dashParser.parse(manifest_prop);
7595
let audioAdaptationsArray = dashManifestModel.getAdaptationsForType(parsedMpd, 0, 'audio');
@@ -118,4 +138,3 @@ describe('DashParser', function () {
118138
})
119139

120140

121-

test/unit/test/mss/mss.parser.MssParser.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ describe('MssParser', function () {
5959
});
6060

6161
it('should throw an error when parse is called with invalid smooth data', function () {
62-
expect(mssParser.parse.bind('<SmoothStreamingMedia')).to.be.throw('parsing the manifest failed');
62+
expect(() => mssParser.parse('<SmoothStreamingMedia')).to.throw('parsing the manifest failed');
6363
});
6464

6565
it('should map mss subtype to dash role', async () => {

test/unit/test/streaming/streaming.Stream.js

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -90,11 +90,9 @@ describe('Stream', function () {
9090
expect(processors).to.be.empty; // jshint ignore:line
9191
});
9292

93-
it('should trigger MANIFEST_ERROR_ID_NOSTREAMS_CODE error when setMediaSource is called but streamProcessors array is empty', () => {
94-
stream.setMediaSource()
95-
.then(() => {
96-
expect(errHandlerMock.errorCode).to.be.equal(Errors.MANIFEST_ERROR_ID_NOSTREAMS_CODE); // jshint ignore:line
97-
})
93+
it('should trigger MANIFEST_ERROR_ID_NOSTREAMS_CODE error when setMediaSource is called but streamProcessors array is empty', async () => {
94+
await stream.setMediaSource();
95+
expect(errHandlerMock.errorCode).to.be.equal(Errors.MANIFEST_ERROR_ID_NOSTREAMS_CODE); // jshint ignore:line
9896
});
9997

10098
it('should return an null when getId is called but streamInfo attribute is null or undefined', () => {

0 commit comments

Comments
 (0)