Staticcacher is currently not covered by any tests. We can unit test it by stubbing sw-cache-helper. We can also add a couple of integration tests between Staticcacher and sw-cache-helper

WIP

I have a set of unit tests running. To check the integration with the Cache helper[1], I'd need to be able to use importScript() to import the NodeJS module? Do you see a way to do it, Salva? [1] https://github.com/arcturus/sw-cache-helper

(In reply to Johan Lorenzo [:jlorenzo] (QA) from comment #2) > I have a set of unit tests running. To check the integration with the Cache > helper[1], I'd need to be able to use importScript() to import the NodeJS > module? Do you see a way to do it, Salva? > > [1] https://github.com/arcturus/sw-cache-helper Oh. I see your point here. I realized Francisco used browserify to convert node modules into "browser" modules. I don't know if we can provide a general solution but let me find an specific one to workaround.

Well, the problem is as follows. `importScript()` runs the foreign code in the current context so all the global variables are automatically exposed in the service worker global scope. The problem is sw-cache-helper is using "strict mode" which checks undefined variables and only allow some well known globals to be directly accessed. For node, `module` is a well known global but for JS, it makes no sense... yet. So, here is the solution (not tried): before using `importScript()`, at the initialization of the test, create a sw global object named module: ```js self.module = {}; ``` Now you should be able to use `importScript()` without throwing as module exists. The CacheHelper class should live inside `self.module.exports` now. Hope it helps. (It should be great if Bugzilla integrates Markdown by the way)

I tried this solution, without success. Here's the error message I get: > Chrome 44.0.2391 (Linux) Static cacher onInstall() should add the files to the cache FAILED > TypeError: Cannot read property 'getDefaultCache' of undefined It doesn't seem like the Node module is correctly imported. What do you think, Salva?

Btw, I also tried to add '/base/node_modules/sw-cache-helper/index.js' to SW_TESTS in sw-tests.js. It didn't help either.

Johan. Upload the code that is not working for you and add me to your repository as a collaborator and I will take a look. Thanks!

I created the node-import-issue branch on my forked repo[1], and you should be now a collaborator of it. [1] https://github.com/JohanLorenzo/serviceworkerware/tree/node-import-issue

Well, it's not the smartest way of do it [1], but it works. We can talk about how to improve the solution. [1] https://github.com/JohanLorenzo/serviceworkerware/commit/fcaab08ccbe758b135e0f1521c6a426b9b8b7b24

Comment on attachment 8599861 [details] PR For the record, :francisco and :salva agreed on merging the cache helper back to SWW. In this PR, I only did it for the staticcacher, as it was the only file that was impacted by this bug. I don't have any issues on Chrome but I can't get the test executed on today's Firefox nightly. Karma keeps the tests in execution mode. Salva, how can I see what's wrong with it?

(In reply to Johan Lorenzo [:jlorenzo] (QA) from comment #10) > Comment on attachment 8599861 [details] > PR > > For the record, :francisco and :salva agreed on merging the cache helper > back to SWW. > > In this PR, I only did it for the staticcacher, as it was the only file that > was impacted by this bug. > > I don't have any issues on Chrome but I can't get the test executed on > today's Firefox nightly. Karma keeps the tests in execution mode. Salva, how > can I see what's wrong with it? Johan, good work. Take a look at my comments on GitHub and ask for my review again. I will try to find the problems on Firefox when implementing bug #1165860.

Comment on attachment 8599861 [details] PR Thanks. It's now ready for another review.

Comment on attachment 8599861 [details] PR Please, rebase and take into account this changes I introduce to ehance test isolation: https://github.com/lodr/serviceworkerware/commit/5f3aff6f279d9b650e3b9a80a8c87b594899e8c0 Ask for my r once you're done but it's almost done.

After applying the commit, I still have 8/44 test failures. I'll wait until bug 1165860 lands to clean up this patch.

Hey, Johan, bug 1166860 is done. Can you rebase? Thanks!

Thanks for letting me know. I currently have 8 tests failing. I need to investigate deeper. I'll be working on them this week.

Comment on attachment 8599861 [details] PR Got the tests back passing. The changes due to the rebase are in the last commit.

Comment on attachment 8599861 [details] PR Thank you Johan. Excellent work! You can merge.

master: 5d55085c08cbe5e87ef0f96d2bf9a63aab5041db

As NGA Program Manager suggested, let's replace the NGA-X milestones with FxOS-Sx ones (more generic ones), once Bug 1174794 has already landed