for #26: refactor (some) filterRequest code into modules and test

[groovecoder]

This doesn't do everything, but it helps ...

• New allowRequest function in a new requests module covered by tests
• New hostInBlocklist function in lists module covered by tests
• New hostInEntity function in lists module covered by tests using new entitylist-fixture.json
• New initTestPilotPingChannel function in background.js to reduce window/global abuse/dependency
• Move allHosts function from canonicalize module to lists module where it's used more

Total of 8 new tests with 14 new assertions.

[groovecoder]

@rpl - r?

Rather than explicitly inject window dependency into functions or abuse global, I decided to leave browser window dependencies in background.js and move code out of background.js into other modules that can be tested. So code in background.js is becoming the only code that relies on browser context globals (fetch, BroadcastChannel, browser), and I'm trying to make it as small as possible.

Does that strategy make sense (for now™)?

[rpl]

@groovecoder absolutely, this strategy sounds great to me
(I didn't mention it before because I wrongly thought that you wanted to test the BroadcastChannel usage as well :-))

I'll look the rest of these patches asap.

[rpl]

@groovecoder This inline comment sounds interesting, may I ask you some additional details about this?
(e.g. some additional info about the scenario and the issue)

[rpl]

@groovecoder 👍

This refactoring is going great! the background page is still pretty big, and big chunks of what the background page is doing needs more testing, but we are getting there!

We are now very near to be able to fully pass eslint checks once this PR will be merged, I'm going to open a new PR which adds eslint to this repo, it can be helpful to keep the current conventions applied over the time (or enforce more conventions over the time).

[rpl]

@groovecoder nevermind, you can ignore the part of the above comment related to eslint, I just saw that standard is running on travis as part of the lint script in the package.json (it doesn't check everything that eslint checks, e.g. limiting process and console usages to only defined places in the code, but it is enough to enforce the most common style rules)

[groovecoder]

Thanks @rpl. May test BroadcastChannel usage in the future, but for now I'm more interested in thoroughly testing the blocking logic. I've honestly forgotten why I (had to?) set the active tab ID to 1. Probably just something I did early before I had read much about the web extension events for doing so ...

[groovecoder]

In fact, I'm almost sure that hack can be fixed in #76 where I properly use a window event handler to assign currentActiveTabID. I remember early on I didn't know how to do that for the first window opened. I'll fix the HACK in that PR.