Issue 3675 - Add some tests for ContentBlockerList

Issue 3675 - Add some tests for ContentBlockerList

[kzar, 2016-05-25 16:20:54]

Patch Set 1

https://codereview.adblockplus.org/29344540/diff/29344541/lib/abp2blocklist.js
File lib/abp2blocklist.js (right):

https://codereview.adblockplus.org/29344540/diff/29344541/lib/abp2blocklist.j...
lib/abp2blocklist.js:252: if (!(filter.contentType & whitelistableRequestTypes))
Fixed this bug in the process of writing the tests :)

https://codereview.adblockplus.org/29344540/diff/29344541/package.json
File package.json (right):

https://codereview.adblockplus.org/29344540/diff/29344541/package.json#newcode8
package.json:8: "nodeunit": "0.9.1"
(Same thing we use for the adblockpluscore tests.)

[Sebastian Noack, 2016-05-25 17:16:33]

Awesome, finnally tests. The code looks good too.

https://codereview.adblockplus.org/29344540/diff/29344541/lib/abp2blocklist.js
File lib/abp2blocklist.js (right):

https://codereview.adblockplus.org/29344540/diff/29344541/lib/abp2blocklist.j...
lib/abp2blocklist.js:252: if (!(filter.contentType & whitelistableRequestTypes))
On 2016/05/25 16:20:54, kzar wrote:
> Fixed this bug in the process of writing the tests :)

Awesome, but how will that fix ever be tested, or mentioned in the release
announcements if there is no issue to track it? ;)

https://codereview.adblockplus.org/29344540/diff/29344541/package.json
File package.json (right):

https://codereview.adblockplus.org/29344540/diff/29344541/package.json#newcode8
package.json:8: "nodeunit": "0.9.1"
On 2016/05/25 16:20:54, kzar wrote:
> (Same thing we use for the adblockpluscore tests.)

I didn't even know. But I guess it makes sense then to use it here as well.
Perhaps update the issue description explaining that nodeunit should be used as
it's already used in adblockpluscore.

[kzar, 2016-05-25 18:31:55]

Patch Set 2 : Removed #4072 fix

https://codereview.adblockplus.org/29344540/diff/29344541/lib/abp2blocklist.js
File lib/abp2blocklist.js (right):

https://codereview.adblockplus.org/29344540/diff/29344541/lib/abp2blocklist.j...
lib/abp2blocklist.js:252: if (!(filter.contentType & whitelistableRequestTypes))
On 2016/05/25 17:16:33, Sebastian Noack wrote:
> On 2016/05/25 16:20:54, kzar wrote:
> > Fixed this bug in the process of writing the tests :)
> 
> Awesome, but how will that fix ever be tested, or mentioned in the release
> announcements if there is no issue to track it? ;)

Good point, created this issue https://issues.adblockplus.org/ticket/4072

https://codereview.adblockplus.org/29344540/diff/29344541/package.json
File package.json (right):

https://codereview.adblockplus.org/29344540/diff/29344541/package.json#newcode8
package.json:8: "nodeunit": "0.9.1"
On 2016/05/25 17:16:33, Sebastian Noack wrote:
> On 2016/05/25 16:20:54, kzar wrote:
> > (Same thing we use for the adblockpluscore tests.)
> 
> I didn't even know. But I guess it makes sense then to use it here as well.
> Perhaps update the issue description explaining that nodeunit should be used
as
> it's already used in adblockpluscore.

Done.

[Sebastian Noack, 2016-05-26 00:34:18]

LGTM