Support bi-directional testing

[amireh]

This patch, although a bit obtrusive, tunes the launcher so that it allows us to require a main-process file so that our renderer tests that rely on back-end communication can be tested.

I'm not sure if this falls out of the intended scope of this project, but my options were either go with this or switch to something like Capybara which I have no intention of doing. If you approve, I can look into writing tests for the new functionality.

Example

For example, suppose I need to test the following function:

// @file: renderer/module.js
const { ipcRenderer } = require('electron');

exports.doSomethingInBackground = function(done) {
  ipcRenderer.once('SOME_CHANNEL_RC', function(event, payload) {
    done(null, payload);
  });

  ipcRenderer.send('SOME_CHANNEL', { datum: 1 });
};

In my mocha test:

describe('MyModule', function() {
  it('can do something in the background', function(done) {
    myModule.doSomethingInBackground(function(err, result) {
      if (err) {
        done(err);
      }
      else {
        assert.equal(result.status, 200);
        done();
      }
    });
  });
});

Where my back-end script looks like this:

// @file: main/module.js
const { ipcMain } = require('electron');

ipcMain.on('SOME_CHANNEL', function(event, payload) {
  event.sender.send('SOME_CHANNEL_RC', { status: 200 });
});

With this patch, I am able to include "main/module.js" in the main process context and directly test the side-effects of my renderer/module.js tests.

Changes

The patch adds three new (optional) launcher options:

• devTools: Boolean - for showing the dev tools panel with the electron BrowserWindow
• entry: String - the absolute path of the main entry file to require
• browserWindowOptions: Object - custom constructor options to pass to our BrowserWindow() call the launcher makes

Doing this with Mocha & Webpack

This is my current stack so I'll explain how it can work. The important bit is to feed Karma two separate entry files; one that gets processed by the electron preprocessor that this plugin provides, and another that gets processed by the "webpack" pre-processor provided by karma-webpack.

Actually, I've made a gist of the config with some annotations which you can view here:

https://gist.github.com/amireh/2fc8f14d503ac9d1298f98d024ed9511

[twolfson]

Thanks for the PR! This seems promising but is combining a lot of different features whereas we prefer 1 PR for 1 feature. Can we split up these 3 features into 3 PRs? I definitely have more questions about devTools and browserWindowOptions

[amireh]

Agreed. I was in fact going to withdraw some of those changes after using them for the day. Maybe you can help me out, let me explain.

The reason I opted for exposing the BrowserWindow constructor options and for using runInParent is because Mocha's errors would get "swallowed up" and be served by Karma as the dreaded Script Error: .0junk error. The only way for me to tell what the error was was to inspect the console tab of the devtools panel.

I remember seeing this error in plain Karma too and it had to do with cross-frame policies. With the Chrome launcher, it was possible to get around it by supplying the (chromium) --disable-web-security flag for the launcher so I was thinking I could do the same thing here with Electron's webkit/blink/something instance. But apparently that wasn't having an effect (BrowserWindow.options.webPreferences.webSecurity = false), and what did have an effect was setting runInParent: true which evidently, as you outlined, brings about a heap of issues of a different kind.

Do you have any insight on how we can get proper error reporting? Is this something only I am seeing?

I've also seen a different issue with sporadic browser disconnects although the Electron container would seem to be alive and only by doing manual refreshes there would Karma re-establish its socket connection again. Once I have the time I will be looking into it.

Overall, I definitely think that this (not the patch, but the project) can be tuned to become a full-stack testing environment and I am grateful for what it currently provides. At least, with all the manual F5s and restarts aside, I did get the job done and called it a day, so thank you. :)

[twolfson]

I don't think I've ever encountered the Script Error: .0 you're mentioning. I have gotten Karma swallowing console.log but that was due to a recent change in its log level handling (i.e. it treats console.log like a LOG_DEBUG iirc). Can you double check your logLevel setting for that?

With respect to cross-frame issues, karma-electron executes based off of an http:// URL so it's still subject to the same restrictions as a normal browser (e.g. can't load file://, can load but not access resources from other domains)

With respect to socket disconnects, I think I've gotten those if my test suite does something obscure like infinite process.nextTick loop but I try to avoid those when possible

With respect to full stack testing, there already is a project for full stack testing of Electron and this project is mostly for renderer only testing. We document this in the README, it's Selenium and WebDriver. Here's the official documentation:

https://github.com/electron/electron/blob/v1.6.7/docs/tutorial/using-selenium-and-webdriver.md

I'm okay with adding require support for other things to add in IPC responses or similar but we'll never be able to touch things like config management since the test hooks are all in the renderer process.

I should also note that the suggested IPC responses will work great for a single set of fixed responses but when people want to do something like swap in/out fixture responses, we'll probably be out of luck. In that scenario, I'll probably point someone to something like mocks for WebSockets (since they're so similar). Here's some example code:

describe('An Application loading a valid config', function () {
    ipcFixtures.load(['config#valid']);

    it('receives a config', function () {
      var config = ipc.sendSync('get-config');
      expect(config).to.have.property('foo');
    });
});

describe('An Application loading an invalid config', function () {
    ipcFixtures.load(['config#invalid']);

    it('receives a config', function () {
      var config = ipc.sendSync('get-config');
      expect(config).to.not.have.property('foo');
    });
});

[amireh]

@twolfson I've amended the patch as suggested, now it only provides the entry file functionality and is backed up by tests.

[twolfson]

This looks great, there are a bunch of one-offs I want to fix (e.g. rename entry to require for vocabulary consistency, simplify tests to do IPC sync only) but I'll land this and iterate on those myself. I'll comment here when it's landed/released

[twolfson]

This has been merged/released in 5.2.0. Thanks again for your work on this!

[amireh]

No problem.

I don't mind the renames / styling changes since it's your project after all, but I'm not sure I feel appreciative of removing those tests :confused:. Why were they removed? To me it seems that you could've just added the other test to cover synchronous messaging and leave the other two in place (one for global support and the other for async communication.)

[twolfson]

With the tests, we only need 1 to verify we are loading the file at all. Anything else (i.e. verifying globals work, both async and sync messaging work, any other functionality of Electron) is testing Electron itself and unrelated to our library

I removed them due to adding unnecessary code to maintain (e.g. if Electron changes their syntax or a developer is debugging and they need to read through those lines)