Skip to content

Commit 6a2bd3a

Browse files
committed
Add tests and fix bugs found
1 parent 80145ca commit 6a2bd3a

9 files changed

Lines changed: 241 additions & 45 deletions

Gruntfile.coffee

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,7 @@ module.exports = (grunt) ->
174174
'node_modules/jasmine-jquery/lib/jasmine-jquery.js'
175175
'node_modules/sinon/pkg/sinon.js'
176176
'node_modules/jasmine-sinon/lib/jasmine-sinon.js'
177+
'node_modules/promise-polyfill/promise.js' # TODO: remove when phantomjs supports promises
177178
]
178179
template: require('grunt-template-jasmine-istanbul')
179180
templateOptions:
@@ -189,6 +190,7 @@ module.exports = (grunt) ->
189190
'node_modules/jasmine-jquery/lib/jasmine-jquery.js'
190191
'node_modules/sinon/pkg/sinon.js'
191192
'node_modules/jasmine-sinon/lib/jasmine-sinon.js'
193+
'node_modules/promise-polyfill/promise.js' # TODO: remove when phantomjs supports promises
192194
]
193195
template: require('grunt-template-jasmine-istanbul')
194196
templateOptions:

dist/CommentCoreLibrary.js

Lines changed: 25 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1271,6 +1271,10 @@ var CommentProvider = (function () {
12711271
return this.applyParsersList(data, type);
12721272
}.bind(this)));
12731273
}
1274+
if (promises.length === 0) {
1275+
// No static loaders
1276+
return Promise.resolve([]);
1277+
}
12741278
return Promise.race(promises).then(function (commentList) {
12751279
for (var i = 0; i < this._targets.length; i++) {
12761280
this._targets[i].load(commentList);
@@ -1293,14 +1297,14 @@ var CommentProvider = (function () {
12931297
return this.load().then(function (commentList) {
12941298
// Bind the dynamic sources
12951299
for (var type in this._dynamicSources) {
1296-
this._dynamicSources[type].foreach(function (source) {
1300+
this._dynamicSources[type].forEach(function (source) {
12971301
source.addEventListener('receive', function (data) {
12981302
for (var i = 0; i < this._targets.length; i++) {
12991303
this._targets[i].send(
13001304
this.applyParserOne(data, type));
13011305
}
13021306
}.bind(this));
1303-
s}.bind(this));
1307+
}.bind(this));
13041308
}
13051309
return Promise.resolve(commentList);
13061310
}.bind(this));
@@ -1398,10 +1402,6 @@ var BilibiliFormat = (function () {
13981402
// Probably not XML
13991403
return null;
14001404
}
1401-
if (!elem.childNodes[0]) {
1402-
// Not a comment or nested comment, skip
1403-
return null;
1404-
}
14051405
var text = elem.textContent;
14061406
var comment = {};
14071407
comment.stime = Math.round(parseFloat(params[0])*1000);
@@ -1884,15 +1884,15 @@ var AcfunFormat = (function () {
18841884

18851885
var CommonDanmakuFormat = (function () {
18861886
var CommonDanmakuFormat = {};
1887-
var _check = function () {
1887+
var _check = function (comment) {
18881888
// Sanity check to see if we should be parsing these comments or not
1889-
if (comment.mode !== "number"|| typeof comment.stime !== "number") {
1889+
if (typeof comment.mode !== 'number' || typeof comment.stime !== 'number') {
18901890
return false;
18911891
}
1892-
if (comment.mode === 8 && !(typeof comment.code === "string")) {
1892+
if (comment.mode === 8 && !(typeof comment.code === 'string')) {
18931893
return false;
18941894
}
1895-
if (typeof comment.text !== "string") {
1895+
if (typeof comment.text !== 'string') {
18961896
return false;
18971897
}
18981898
return true;
@@ -1912,16 +1912,24 @@ var CommonDanmakuFormat = (function () {
19121912
CommonDanmakuFormat.XMLParser = function () { };
19131913
CommonDanmakuFormat.XMLParser.prototype.parseOne = function (comment) {
19141914
var data = {}
1915-
data.stime = parseInt(comment.getAttribute('stime'));
1916-
data.mode = parseInt(comment.getAttribute('mode'));
1917-
data.size = parseInt(comment.getAttribute('size'));
1918-
data.color = parseInt(comment.getAttribute('color'));
1919-
data.text = comment.textContent;
1915+
try {
1916+
data.stime = parseInt(comment.getAttribute('stime'));
1917+
data.mode = parseInt(comment.getAttribute('mode'));
1918+
data.size = parseInt(comment.getAttribute('size'));
1919+
data.color = parseInt(comment.getAttribute('color'));
1920+
data.text = comment.textContent;
1921+
} catch (e) {
1922+
return null;
1923+
}
19201924
return data;
19211925
};
19221926

1923-
CommonDanmakuFormat.XMLParser.prototype.parseMany = function (comments) {
1924-
var comments = comments.getElementsByTagName('comment');
1927+
CommonDanmakuFormat.XMLParser.prototype.parseMany = function (commentsElem) {
1928+
try {
1929+
var comments = commentsElem.getElementsByTagName('comment');
1930+
} catch (e) {
1931+
return null;
1932+
}
19251933
var commentList = [];
19261934
for (var i = 0; i < comments.length; i++) {
19271935
var comment = this.parseOne(comments[i]);

dist/CommentCoreLibrary.min.js

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
"jasmine-jquery": "^2.0.0",
4141
"sinon": "^1.17.6",
4242
"jasmine-sinon": "^0.4.0",
43+
"promise-polyfill": "^6.0.2",
4344
"load-grunt-tasks": "^3.5.2"
4445
}
4546
}

spec/CommentProvider_spec.coffee

Lines changed: 98 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
'use strict'
22

33
describe 'CommentProvider', ->
4-
beforeEach ->
5-
# TODO: remove promises api polyfill when phantomjs supports it
6-
window.Promise = (f) ->
7-
# Don't actually call f
8-
window.Promise.prototype.then = (f) -> this
9-
window.Promise.prototype.catch = (f) -> this
10-
window.Promise.resolve = (f) -> new Promise()
4+
xhr = null
5+
phonyParser = null
6+
beforeAll ->
7+
xhr = sinon.useFakeXMLHttpRequest()
8+
phonyParser =
9+
parseOne: (item) -> item
10+
parseMany: (items) -> items
1111

1212
it 'has constants', ->
1313
expect(CommentProvider.SOURCE_XML).toBe 'XML'
@@ -61,7 +61,7 @@ describe 'CommentProvider', ->
6161
describe '.addStaticSource', ->
6262
it 'accepts static source', ->
6363
promise = {}
64-
provider.addStaticSource(promise, CommentProvider.SOURCE_XML)
64+
provider.addStaticSource promise, CommentProvider.SOURCE_XML
6565
expect(provider._staticSources[CommentProvider.SOURCE_XML][0]).toBe promise
6666

6767
it 'rejects if provider is shut down', ->
@@ -70,6 +70,94 @@ describe 'CommentProvider', ->
7070
expect( => provider.addStaticSource({}, CommentProvider.SOURCE_XML)).toThrow()
7171

7272
describe '.addDynamicSource', ->
73+
it 'accepts dynamic source', ->
74+
dynamicSource =
75+
addEventListner: sinon.stub()
76+
provider.addDynamicSource dynamicSource, CommentProvider.SOURCE_XML
77+
expect(provider._dynamicSources[CommentProvider.SOURCE_XML][0]).toBe dynamicSource
7378

74-
75-
79+
it 'rejects if provider is shut down', ->
80+
provider = new CommentProvider()
81+
provider.destroy()
82+
expect( => provider.addDynamicSource({}, CommentProvider.SOURCE_XML)).toThrow()
83+
84+
describe '.addParser', ->
85+
it 'accepts parser', ->
86+
parser = {}
87+
provider.addParser parser, CommentProvider.SOURCE_JSON
88+
expect(provider._parsers[CommentProvider.SOURCE_JSON][0]).toBe parser
89+
90+
it 'rejects if provider is shut down', ->
91+
provider = new CommentProvider()
92+
provider.destroy()
93+
expect( => provider.addParser({}, CommentProvider.SOURCE_XML)).toThrow()
94+
95+
describe '.addTarget', ->
96+
commentManager = null
97+
beforeAll ->
98+
commentManager = new CommentManager(document.createElement 'div')
99+
100+
it 'accepts target', ->
101+
provider.addTarget commentManager, CommentProvider.SOURCE_JSON
102+
expect(provider._targets.length).toBe 1
103+
expect(provider._targets[0]).toBe commentManager
104+
105+
it 'rejects if target is not CommentManager', ->
106+
expect( => provider.addTarget {}).toThrow()
107+
108+
it 'rejects if provider is shut down', ->
109+
provider = new CommentProvider()
110+
provider.destroy()
111+
expect( => provider.addTarget(commentManager)).toThrow()
112+
113+
describe '.load', ->
114+
it 'requests static sources', (done) ->
115+
provider.addStaticSource (Promise.resolve 'Foo'), CommentProvider.SOURCE_TEXT
116+
provider.addParser phonyParser, CommentProvider.SOURCE_TEXT
117+
spy = sinon.spy phonyParser, 'parseMany'
118+
provider.load().then () ->
119+
expect(spy).toHaveBeenCalledWith 'Foo'
120+
done()
121+
122+
it 'fails if provider is shut down', ->
123+
provider = new CommentProvider()
124+
provider.destroy()
125+
expect( => provider.load()).toThrow()
126+
127+
it 'fails if no sources are available', (done) ->
128+
provider.addStaticSource (Promise.reject 'Error'), CommentProvider.SOURCE_TEXT
129+
provider.load().catch (e) ->
130+
expect(e).toBe 'Error'
131+
done()
132+
133+
describe '.start', ->
134+
dynamicSource = null
135+
spy = null
136+
137+
beforeEach ->
138+
dynamicSource =
139+
addEventListener: () ->
140+
spy = sinon.spy dynamicSource, "addEventListener"
141+
142+
it 'calls load', ->
143+
loadspy = sinon.spy provider, 'load'
144+
provider.start()
145+
expect(loadspy).toHaveBeenCalled true
146+
147+
it 'binds dynamic sources', (done) ->
148+
provider.addDynamicSource dynamicSource, CommentProvider.SOURCE_XML
149+
provider.start().then () ->
150+
expect(spy).toHaveBeenCalled true
151+
done()
152+
153+
it 'does not bind dynamic if the static sources fail', (done) ->
154+
provider.addDynamicSource dynamicSource, CommentProvider.SOURCE_XML
155+
provider.addStaticSource (Promise.reject 'Error'), CommentProvider.SOURCE_XML
156+
provider.start().catch () ->
157+
expect(spy.callCount).toBe 0
158+
done()
159+
160+
describe '.destory', ->
161+
it 'sets destroyed flag', ->
162+
provider.destroy()
163+
expect(provider._destroyed).toBe true
Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
'use strict'
2+
describe 'CommonDanmakuFormat', ->
3+
it 'provides xml parser', ->
4+
expect(typeof CommonDanmakuFormat.XMLParser).toBe 'function'
5+
6+
it 'provides json parser', ->
7+
expect(typeof CommonDanmakuFormat.JSONParser).toBe 'function'
8+
9+
describe '.XMLParser', ->
10+
parser = null
11+
elem = null
12+
13+
beforeAll ->
14+
elem = document.createElement 'comment'
15+
elem.setAttribute 'mode', '1'
16+
elem.setAttribute 'size', '25'
17+
elem.setAttribute 'color', '255'
18+
elem.setAttribute 'stime', '1100'
19+
elem.textContent = 'Test'
20+
21+
beforeEach ->
22+
parser = new CommonDanmakuFormat.XMLParser()
23+
24+
it 'only accepts xml documents', ->
25+
expect(parser.parseOne "foo").toBe null
26+
expect(parser.parseMany "foo").toBe null
27+
28+
it 'can parse one', ->
29+
expect(parser.parseOne elem).toEqual
30+
'stime': 1100,
31+
'mode': 1,
32+
'size': 25,
33+
'color': 255,
34+
'text': 'Test'
35+
36+
it 'can parse many', ->
37+
container = document.createElement 'comments'
38+
container.appendChild elem
39+
expect(parser.parseMany container).toEqual [
40+
{
41+
'stime': 1100,
42+
'mode': 1,
43+
'size': 25,
44+
'color': 255,
45+
'text': 'Test'
46+
}
47+
]
48+
49+
describe '.JSONParser', ->
50+
parser = null
51+
dataValid =
52+
mode: 1
53+
text: "Foo"
54+
stime: 0
55+
dataNoMode =
56+
stime: 1
57+
text: "Foo"
58+
dataNoStime =
59+
mode: 1
60+
text: "Foo"
61+
dataMode8 =
62+
mode: 8
63+
stime: 0
64+
text: "This should be code"
65+
dataNoText =
66+
mode: 1
67+
stime: 0
68+
69+
beforeEach ->
70+
parser = new CommonDanmakuFormat.JSONParser()
71+
72+
it 'can parse one', ->
73+
expect(parser.parseOne dataValid).toBe dataValid
74+
75+
it 'rejects no mode', ->
76+
expect(parser.parseOne dataNoMode).toBe null
77+
78+
it 'rejects no stime', ->
79+
expect(parser.parseOne dataNoStime).toBe null
80+
81+
it 'rejects mode 8 no code', ->
82+
expect(parser.parseOne dataMode8).toBe null
83+
84+
it 'rejects no text', ->
85+
expect(parser.parseOne dataNoText).toBe null
86+
87+
it 'can parse many', ->
88+
expect(parser.parseMany [dataValid]).toEqual [dataValid]
89+

src/CommentProvider.js

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,10 @@ var CommentProvider = (function () {
273273
return this.applyParsersList(data, type);
274274
}.bind(this)));
275275
}
276+
if (promises.length === 0) {
277+
// No static loaders
278+
return Promise.resolve([]);
279+
}
276280
return Promise.race(promises).then(function (commentList) {
277281
for (var i = 0; i < this._targets.length; i++) {
278282
this._targets[i].load(commentList);
@@ -295,14 +299,14 @@ var CommentProvider = (function () {
295299
return this.load().then(function (commentList) {
296300
// Bind the dynamic sources
297301
for (var type in this._dynamicSources) {
298-
this._dynamicSources[type].foreach(function (source) {
302+
this._dynamicSources[type].forEach(function (source) {
299303
source.addEventListener('receive', function (data) {
300304
for (var i = 0; i < this._targets.length; i++) {
301305
this._targets[i].send(
302306
this.applyParserOne(data, type));
303307
}
304308
}.bind(this));
305-
s}.bind(this));
309+
}.bind(this));
306310
}
307311
return Promise.resolve(commentList);
308312
}.bind(this));

src/parsers/BilibiliFormat.js

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,6 @@ var BilibiliFormat = (function () {
5555
// Probably not XML
5656
return null;
5757
}
58-
if (!elem.childNodes[0]) {
59-
// Not a comment or nested comment, skip
60-
return null;
61-
}
6258
var text = elem.textContent;
6359
var comment = {};
6460
comment.stime = Math.round(parseFloat(params[0])*1000);

0 commit comments

Comments
 (0)