karma-runner / karma

Spectacular Test Runner for JavaScript
http://karma-runner.github.io
MIT License
11.96k stars 1.71k forks source link

Add support for testing CommonJS format modules #552

Closed necolas closed 11 years ago

necolas commented 11 years ago

I might have missed something, but I haven't found a way to test CommonJS format modules.

If you author modules this way, and then use a tool like r.js to convert them to AMD modules and produce an asset bundle for dev (which is then optimized for production), how can you test those modules with Karma?

Thanks

jamesshore commented 11 years ago

+1

I'm using Browserify in my front-end code and this is necessary for me to do good cross-browser testing. I'm currently loading the Browserify bundle, which allows my tests to run, but it doesn't produce good stack traces. (I haven't been able to get source maps to work, even on Chrome, and they don't work on all the browsers I support anyway.)

jamesshore commented 11 years ago

I've been thinking about this and I think a solution will take a two-part approach. First, we need an implementation of require() so the CommonJS require function is supported. We can do that in a test_main.js in a similar way as described for RequireJS support.

Second, we need a way of getting the module and exports variables into each file. We could do that by using a preprocessor to put a wrapper around each file, something like this:

;function(require, module, exports) {
  // original file here
}(window.__commonjsbridge__.require,
window.__commonjsbridge__.module,
window.__commonjsbridge__.module.exports);

There may be a better way; that's just my initial thoughts after looking at this problem for an hour or so. I'm going to keep working on it--suggestions or solutions welcome.

necolas commented 11 years ago

I've been writing CommonJS-style modules and running a watch task that passes them through the r.js optimizer to produce an AMD bundle. Like you, I'd rather avoid loading the whole bundle if possible but I'm already having to wrap my tests in define() anyway, so I'm not really sure. Is there's an existing approach that most people writing CommonJS modules are taking to test their code with Karma at the moment? Thanks

vojtajina commented 11 years ago

There needs to be a dev workflow without bundling all sources into a single file. (even if you manage source maps to work, this makes it slow, as you need to "recompile" the whole project any time you change any file)

@jamesshore suggestion is one way to go (you might check out https://github.com/montagejs/montage 's require as they are doing this; it's good to make sure you don't mungle line numbers - so the prefix should not add new line).

For Google Closure library, I have a prototype of resolving dependencies on the server (https://github.com/vojtajina/closure-deps-resolver). That's probably what I would try. Preprocessors scan files for dependencies and maintain an in-memory graph. Then, when files are served, the order is resolved based on that dependency graph. It works really well. With Closure it's easier, because goog.require() is noop, if the namespace has already been included. In the case of CJS, we will have to implement some fake APIs that would return the exported stuff.

In addition, you can do things like this: if any of test files contains iit or ddescribe, only these test files are loaded and only the files that are actually required by these tests are sent to the browser. This has a huge impact on speed on huge projects. (the closure plugin does this as well)

jonykrause commented 11 years ago

+1

What I did when using Browserify was wrapping the testcode in a minimal bundle, not pretty cool but worked:

;(function(e,t,n){function i(n,s){if(!t[n]){if(!e[n]){var o=typeof require=="function"&&require;if(!s&&o)return        o(n,!0);if(r)return r(n,!0);throw new Error("Cannot find module '"+n+"'")}var u=t[n]={exports:{}};e[n][0].call(u.exports,function(t){var r=e[n][1][t];return i(r?r:t)},u,u.exports)}return t[n].exports}var r=typeof require=="function"&&require;for(var s=0;s<n.length;s++)i(n[s]);return i})({1:[function(require,module,exports){

// testcode

},{}]},{},[1]);

But I just discovered karma-browserify which may also be interesting for you guys.

necolas commented 11 years ago

Oh nice, thanks :)

jamesshore commented 11 years ago

@jonykrause, thanks for the karma-browserify tip. I took a look at it and it seems like its goal is to solve the problem of slow builds due to rerunning browserify. It didn't look like it tried to solve the stack trace / line number problem. Have you tried it yet? Does it address the stack trace issue?

pluma commented 11 years ago

The Right Thing To Do (tm) would be to use the source maps produced by the bundler. There are many situations where source line numbers won't match the real line numbers, bundling is just one of them. Consider CoffeeScript, LiveScript, TypeScript, Dart or what have you. It may be possible to avoid bundling in some situations, but in many a build step is necessary and source code transformations will occur that break the traces.

I don't think CommonJS support would be sufficient when dealing with browserify, because it also "polyfills" some parts of the node standard library to allow running server code on the client. The build/bundle step does more than just concatenate files.

EDIT: I've added a new issue [#594] because handling source maps doesn't really have anything to do with CommonJS other than that browserify involves both.

jamesshore commented 11 years ago

I've tried karma-browserify and it doesn't resolve the stack trace problem.

jamesshore commented 11 years ago

I've created a bridge for using CommonJS modules with Karma v0.8. It's not meant to handle every case, just the ones I care about. :-) But it supports the essential parts of CommonJS modules--require() and exporting.

Feedback desired. I will probably turn this into an npm module and add Karma 0.9 support at some point in the future.

The bridge works by using a preprocessor to wrap your files in a function that provides require, exports, and module. Then another file (karma_commonjs_bridge.js) provides the glue code and makes sure your files are run.

I'm not aware of any way to add custom preprocessors to Karma 0.8, so using this bridge requires you to edit your Karma installation. Here's how to use it:

exports.Commonjs = require('./preprocessors/Commonjs');

var log = require('../logger').create('preprocess.commonjs');

var CommonJS = function(content, file, basePath, done) {
  log.debug('Processing "%s".', file.originalPath);

    var output =
        "window.__karma__.CJSModules = window.__karma__.CJSModules || {};" +
        "window.__karma__.CJSModules['" + file.originalPath + "'] = function(require, module, exports) {" +
            content +
        "}";

    done(output);
};

// PUBLISH
module.exports = CommonJS;
// Copyright (c) 2013 Titanium I.T. LLC. Licensed under the MIT license.
(function() {
    "use strict";

    var cachedModules = {};

    for (var modulePath in window.__karma__.CJSModules) {
        require(modulePath, modulePath);
    };

    function require(requiringFile, dependency) {
        dependency = normalizePath(requiringFile, dependency);

        // find module
        var moduleFn = window.__karma__.CJSModules[dependency];
        if (moduleFn === undefined) throw new Error("Could not find module '" + dependency + "' from '" + requiringFile + "'");

        // run the module (if necessary)
        var module = cachedModules[dependency];
        if (module === undefined) {
            module = { exports: {} };
            moduleFn(requireFn(dependency), module, module.exports);
            cachedModules[dependency] = module;
        }
        return module.exports;
    };

    function requireFn(basepath) {
        return function(dependency) {
            return require(basepath, dependency);
        };
    };

    function normalizePath(basePath, relativePath) {
        if (isFullPath(relativePath)) return relativePath;
        if (!isFullPath(basePath)) throw new Error("basePath should be full path, but was [" + basePath + "]");

        var baseComponents = basePath.split("/");
        var relativeComponents = relativePath.split("/");

        baseComponents.pop();     // remove file portion of basePath before starting
        while (relativeComponents.length > 0) {
            var nextComponent = relativeComponents.shift();

            if (nextComponent === ".") continue;
            else if (nextComponent === "..") baseComponents.pop();
            else baseComponents.push(nextComponent);
        }
        return baseComponents.join("/");

        function isFullPath(path) {
            var unixFullPath = (path.charAt(0) === "/");
            var windowsFullPath = (path.indexOf(":") !== -1);

            return unixFullPath || windowsFullPath;
        }
    }

}());
preprocessors = {
    "src/client/*.js": "commonjs"
};
files = [
    MOCHA,
    MOCHA_ADAPTER,
    "node_modules/expect.js/expect.js",

    "src/client/**/*.js",
    "build/karma_commonjs_bridge.js"
];

Sorry for the rushed explanation--I hope this is helpful. Please comment, criticize, or ask questions.

vojtajina commented 11 years ago

@jamesshore Very nice!

jamesshore commented 11 years ago

Thanks, @vojtajina. The preprocessor doesn't add any newlines (it's just formatted that way), so stack traces work fine. I do hope to make a Karma 0.9 (or is it 0.10?) plugin once it's released.

Thanks for the note about __karma__. I'll fix that.

vojtajina commented 11 years ago

Oh, you are right, sorry I just skimmed through the code really briefly so missed that...

Yep, we gonna push 0.10 out really soon (just polishing the docs), but it will be pretty much what 0.9.x is now. It's just that we mark it with a "stable" which means only bug fixes will go into 0.10.x and new features / possible breaking changes (hopefully not) will be developed in 0.11 (canary).

On Mon, Jul 29, 2013 at 10:10 PM, James Shore notifications@github.comwrote:

Thanks, @vojtajina https://github.com/vojtajina. The preprocessor doesn't have add any newlines (it's just formatted that way), so stack traces work fine. I do hope to make a Karma 0.9 (or is it 0.10?) plugin once it's released.

Thanks for the note about karma. I'll fix that.

— Reply to this email directly or view it on GitHubhttps://github.com/karma-runner/karma/issues/552#issuecomment-21770040 .

azer commented 11 years ago

Fox (https://github.com/azer/fox) has support for this with source urls for those looking for alternatives. It needs no configuration and runs your tests on both NodeJS, Phantom and browsers. A demo video https://vimeo.com/70852179.

vojtajina commented 11 years ago

@jamesshore Let me know if you need any help with extracting it into a 0.10 plugin...

jamesshore commented 11 years ago

@vojtajina Will do, thanks. I'll probably start working on it in the next week or so.

vojtajina commented 11 years ago

@jamesshore I somehow got to finally refactor Karma's client code to use CommonJS and so I did https://github.com/karma-runner/karma-commonjs (it's mostly your code). You have permissions to push to the repo, so I hope you will make it even better ;-)

That said, I'm closing this issue.

necolas commented 11 years ago

Fantastic! Thanks so much guys.

jamesshore commented 11 years ago

@vojtajina Thanks! I'll be looking at that this week.