Adding support for Electron through karma.

[orther]

I have added support for Electron running tests with the karma-electron-launcher in my fork. Before I make a proper pull request, I'd like advice on handling the creation and loading of a shim file.

Please ignore my current crude implementation as I plan to clean all this code up once I decide the best way forward. Below is the basic outline of how I added support for the Electron shim:

1. I added the shim file in the resources folder library/resources/runners/electron-shims.js
2. To create the temp shim file and get it's path I just copied the doo.core/runner-path! function to the doo.karma namespace.
   1. NOTE: Creating a new temp file for the shim may not be necessary since the content of the shim file are do not get dynamically generated like the karma.config.js file does. The reason I did it this way is because I was just copying how the phantomjs shim file was created.
3. In ->karma-opts function, if Electron is one of the launchers the temp shim file is created using runner-path! and it's path is added as the first item in the "files" list (see karma.clj#L88-L89).
   1. NOTE: Order of the files in the "files" list is important since it determines the order files are loading in the karma.config.js that is create. For a shim, it's likely that it needs first or at least before tests are loaded.

My Questions:

1. Is there is an existing way I can create one off files for karma runners that I should be using? (This would replace the runner-path! function I copied to the doo.karma namespace to create the temp electron-shims.js.)
2. Is it possible for a project using lein-doo to add a file to the top of the "files" array in the karma.config.js file? This would allow projects to add custom shims to karma if wanted.
   1. If yes, is that a better way to support adding the Electron shim?
   2. If no, is that something worth supporting? (even if not ideal for the Electron shim)
3. If specific shims are needed for different karma launchers, can/should we create a different karma.config.js for each unique config based on "files"array? (This could be an advanced feature that is decided on at a later date. I currently don't have a use case for running Electron specific tests in other launchers.)

I know comments in issues #34 and #43 mention the need to come up with a general solution to a more composable karma config. I believe shim support could be solved with that but, I don't know the status of that solution and believe that since this shim is specific to a single launcher it is more acceptable to solve it in a hard coded one off manner if necessary.

NOTE: I'll explain why a shim is needed as comment on this issues shortly.

[orther]

Why a shim is needed for Electron:
Electron provides built-in modules for developing native desktop applications and those modules are accessed by requiring the electron module like so:

// JavaScript example of accessing Electron modules
const electron = require('electron');
const app = electron.app;
const screen = electron.screen;

;; ClojureScript example of accessing Electron modules
(def electron (js/require "electron"))
(def app (.-app (.-remote electron)))
(def screen (.-screen electron))

When running tests with karma in Electron the require function is not available and you will get this error: ReferenceError: require is not defined.

NOTE: You will only get that error if you are using require in the tests or code you're testing. If you are not using require you do NOT need a shim.

After some googling I came across this karma.shim.js which solves the missing require error with this line:

window.require = window.top.require;

[orther]

I'd like to mention that if it makes sense, I can just clean up my current code and make a pull request with working code to support running tests in Electron using karma with the shim.

[bensu]

Hi @orther

This is great work. Thank you for doing this and then taking the time to explain it so well!

Re Outline 2.i It is probably necessary to create the file for the shim dynamically even though the contents are static. When doo executes the tests, the file is in a jar, which can be accessed by a Java process but not by Node. It needs to be copied into the file system so that Node can find it.

Answers:

1. Your solution is good. runner-path! copies an io/resource into tmp and returns the absolute path. Instead of copying the function into doo.karma, move it to doo.utils, and use it from both doo.karma and doo.core.
2. In theory it is possible to add custom shims to doo, but it is also possible to prepend "shims" by using the :foreing-libs option of the compiler. We should add the shims that are absolutely necessary to the runner but not to specific projects. Feature request: add a way to inject javascript references #25 shows why it is not necessary to expose the functionality to users. Your solution is good as it is.
3. I'm not sure I understand. Karma is launched using one karma.conf.js file. To use multiple conf files, you would need multiple karma instances. If you are going to launch electron, then the electron conf file is needed. If that conf file prevents other launchers from working, we would need to launch other karma servers. As far as I can tell, this functionality is not needed.

Also, the Nashorn PR #66 and the Canary PR #73 can be useful reference.

Feel free to submit your changes as a PR and we can keep discussing it there.

Thanks again!

[orther]

Thanks @bensu you're response was exactly what I was looking for.

I am pretty close to having the code in shape for a pull request but I am having a problem with the boot tests not passing. Right now boot loads all the files in src and test directories and runs the test with phantomjs. That will not work for the electron tests/src so I need to exclude specific files. Do you know the best way to do that with boot/ boot-cljs-test? I'm sure I'll be able to figure it out with some banging my head against the wall but I figured I'd ask.

[bensu]

To rephrase: boot is adding src/example/electron.cljs, test/example/electron_runner.cljs, and test/example/electron_test.cljs to the compilation which breaks phantom. ClojureScript normally adds only what is necessary to the build, which allows you to solve the problem through :main. Is that the problem? Then I don't know how to solve it but @crisptrutski might have an idea.

[crisptrutski]

@orther @bensu Just pushed 0.2.1 of boot-cljs-test which fixes usage with karma (and electron)

[bensu]

@orther Any progress? I recently attempted adding karma-slimerjs and karma-phantomjs in this branch which might be useful for this enhancement.

[bensu]

@orther the latest master has now a beta implementation of electron. Would you mind trying it?

It implements the bare minimum from your fork, and if you are up for it we could continue to add work from your fork.

[orther]

@bensu I switched my focus to mostly JS (paid projects) over the last couple months but I've been meaning to get back to CLJS hobby project(s). I welcome this as a catalyst!

I have pulled the master branch and can successfully run the core tests in Electron. I am working on getting the Electron tests from my fork to run/pass. Once I get the Electron tests working I'll let you know and/or make a pull request.

[orther]

I have created a new branch on my fork based on upstream master and added the changes to support electron testing here: https://github.com/orther/doo/tree/electron-test

You can see I added a shim file and build-ids to use it. master...orther:electron-test

[bensu]

Great!

I like the tests but I see that adding the shim implies many other changes. Why is the shim necessary and is it common to all ClojureScript Electron projects?

I try not to add shims if we can avoid it.

[orther]

You can see why the shim is required in my first comment of this issue here: #90 (comment)

It would be great if instead of putting the shim in the library, the require error could be fixed in the (example) project but I haven't found a way to do that. I haven't looked into this for some time so I can't fully remember the details and just copied over what my working solution was from a few months ago. I'll look into this further.

[bensu]

Hi @orther,

You are right, you were perfectly clear about the shim. Sorry for the oversight. After a third look, the shim looks good. It is probably related to how Karma evaluates the test code inside the JavaScript environment, and the need to reach the top window frame where the special Electron objects where added.

The code is ready for a PR. I'll ask you to change is that if you are moving runner-path!, there should be now duplicate left.

[orther]

I'll try to clean this up and create PR tomorrow afternoon.

[orther]

I also wanted to mention that I am going to look into http://electron.atom.io/spectron/ and if it makes sense to support it with doo.