Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I've created this (very) contrived example that demonstrates how we currently organize page specific JS in our app. Basically each page has a matching JS file that uses the module pattern to enclose page specific behaviors. Each module has an init() function that takes control IDs and wires up events, etc.

This has worked fairely well for us, however now we are wanting to add unit tests. Since many of our modules store state in private variables our unit tests end up stomping all over each other.

I think we need to stop using these singleton modules and instead use instances so that we can write better tests. What are some ways we could restructure this type of code so that it would be testable?

Fiddle here: http://jsfiddle.net/joshuapoehls/z4P3R/5/

HTML

<div>
  <input type="file" /><br/>
  <input type="file" /><br/>
  <input type="file" />
</div>
<p>
  <br/>
  <input id="btnUpload" type="button" value="Upload Files" />
  or <a href="#" id="btnClear">Clear files</a>
</p>

<script>
    MyPage.init({
      clearLink: '#btnClear',
      uploadBtn: '#btnUpload',
      fileCtrls: "input[type='file']"
    });
</script>

JS

MyPage = (function() {
    // private members
    var _files = [];
    var _initOptions;

    // public members
    return {
        init: function(options) {
            _initOptions = options;

            $(function() {
                // wire up the events
                $(options.clearLink).click(MyPage.clear);
                $(options.uploadBtn).click(MyPage.doUpload);
                $(options.fileCtrls).change(MyPage.addFile);
            });
        },

        addFile: function() {
            _files.push($(this).val());
        },

        doUpload: function() {
            alert('uploading ' + _files.length + ' files...');
        },

        clear: function() {
            _files = [];
            $(_initOptions.fileCtrls).val('');
        }
    }
})();
share|improve this question
add comment

4 Answers

up vote 1 down vote accepted

Here's my approach to your scenario. Still working through a few things; however, this should give you the general idea.

  1. A new DOM can be injected during testing so that I don't need to rely on the true form if I don't want/need such.

  2. The DOM object in general could be abstracted out to it's own namespace for selector reuse across pages. (I'm still toying with this idea).

  3. The approach adds an object/namespace to the global scope.

Testing HTML

<!DOCTYPE html>
<html>
<head>
    <script src="http://code.jquery.com/jquery-1.6.1.min.js"></script>
    <script src="Script.js"></script>
    <script>
        var testDom = {
            uploadButton: $("<input type='button'>")
        };
        $(function(){
            var page = new MyPage.Behaviors(testDom);
            page.init();

            testDom.uploadButton.trigger("click");
        });
    </script>
</head>
<body>
</body>
</html>

Example Script

var MyPage = function($) {
    var _files
        , Dom = (function($){
                return {
                    clearLink: $("#btnClear")
                    , uploadButton: $("#btnUpload")
                    , filesControl: $("input[type=file]")
                };
            }($))

        ,   Behaviors = function(dom) {         
                if (typeof dom === 'undefined'){
                    dom = Dom;
                }

                function clear() {
                        _files = [];
                        dom.fileControl.val("");
                }

                function doUpload(){
                    alert('uploading ' + _files.length + ' files...');
                }

                function addFile(e){
                    _files.push(e.target.value);
                }

                function bindElement ($element, trigger, callback){
                    if (typeof $element !== 'undefined'){
                        $element.bind(trigger, callback);
                    }
                }

                function init() {
                    _files = [];
                    bindElement(dom.clearLink, "click", clear);
                    bindElement(dom.uploadButton, "click", doUpload);
                    bindElement(dom.fileControl, "click", addFile);
                }

                return {
                    clear: clear
                    , init: init
                    , doUpload: doUpload
                    , addFile: addFile
                };
            }

        return {
            Behaviors: Behaviors
        };
}(jQuery);

On load, the Behaviors is initialized to my test DOM and then the uploadButton's click event is triggered to test the result.

This works when tested in a browser but doesn't with Fiddle currently.

I'm sure there's a better way; however, this would allow you to test your JavaScript in a more controlled method and also removes the reliance on actual markup if you are using something like JsTestDriver.

share|improve this answer
    
mocking the DOM like that seems like a horrible idea. –  Raynos Jun 28 '11 at 16:57
    
If you don't mock the DOM that way, then the only other alternative is to duplicate your page's markup for each feature/page. By abstracting out the DOM, you remove the need to actually have a test Html page. –  JamesEggers Jun 28 '11 at 17:28
    
@JamesEggers what's the difference between your testDom variable and test markup ? –  Raynos Jun 28 '11 at 17:35
    
@Raynos Ultimately, it's the ancillary markup (i.e. Html tag, etc.). The other difference would be I could use JsTestDriver and keep all of my JavaScript and the tests in JavaScript and won't need to have separate Html file for the markup or to house JavaScript tests. –  JamesEggers Jun 28 '11 at 17:50
    
@Raynos Another advantage (in my opinion) of abstracting out the DOM selectors is that it becomes easier to reuse the same selector across pages. May not be a large gain when minified and combined, admittingly; however, may decrease the overall payload size. –  JamesEggers Jun 28 '11 at 17:52
show 2 more comments
MyPage = (function() {
    var Page = {
        addFile: function(e) {
            this._files.push($(e.target).val());
        },

        doUpload: function() {
            alert('uploading ' + this._files.length + ' files...');
        },

        clear: function() {
            this._files = [];
            $(this.options.fileCtrls).val('');
        }
    }

    // public members
    return {
        init: function(options) {
            var o = Object.create(Page, {
                options: { value: options },
                _files: { value: [] }
            });

            $(function() {
                // wire up the events
                options.clearLink.click($.proxy(o.clear, o));
                options.uploadBtn.click($.proxy(o.doUpload, o));
                options.fileCtrls.change($.proxy(o.addFile, o));
            });
        },


    }
})();

Using Object.create

Live Example

ES5-shim for browser support.

As for testing use any old testing framework you like. Use jQuery.sub() to mock out jQuery

share|improve this answer
    
How would you test this? Also, what if you needed to support a browser that doesn't support Object.create (like IE8)? –  JamesEggers Jun 28 '11 at 16:43
    
@JamesEggers you can test using any old framework you want. I would use QUnit personally. Just create a dummy markup on your test page and create new page objects. I tend to have a generict resetMarkup function on my dummy pages for testing purposes. –  Raynos Jun 28 '11 at 16:59
add comment

You have to design an application to be testable. Automated testing does not mean "automatic" testing. All we can do with a unit testing framework (like QUnit) is make assertions on what can be accessed by the framework.

Any "method" you want to unit-test should be accessible outside of its closure or prototype object; (i.e.) it should be a property of the object, not a variable scoped within its function definition. After you call one of these methods in a unit test you can compare its return value or the application state against some expected value or condition. Again the application state you want to inspect has to be represented by public-scope properties too.

The original script example relies heavily on UI event-driven behavior. You could the jQuery trigger function to emulate these events. Here's a refactored version of the app with a possible technique with a QUnit test suite included: (XML declaration and XHTML doctype omitted by the editor)

<html>
<head>
    <title>Refactored App with QUnit Example</title>
    <link href="http://code.jquery.com/qunit/git/qunit.css" media="screen" type="text/css" rel="stylesheet" />
</head>
<body>

    <!-- original app markup -->
    <div>
        <input type="file" /><br/>
        <input type="file" /><br/>
        <input type="file" />
    </div>
    <p>
        <br/>
        <input id="btnUpload" type="button" value="Upload Files" />
        or <a href="#" id="btnClear">Clear files</a>
    </p>

    <!-- QUnit boilerplate: -->
    <h1 id="qunit-header">Example QUnit Test Suite</h1>
    <h2 id="qunit-banner"></h2>
    <div id="qunit-testrunner-toolbar"></div>
    <h2 id="qunit-userAgent"></h2>
    <ol id="qunit-tests"></ol>
    <div id="qunit-fixture"></div>

    <script type="text/javascript" src="http://code.jquery.com/jquery-latest.js"></script>
    <script type="text/javascript" src="http://code.jquery.com/qunit/git/qunit.js"></script>
    <script type="text/javascript">

    // original app, refactored:

    var App = (function (jquery) {

        var $ = jquery,
            app = this;

        app.files = [];

        app.initOptions = null;

        app.addFile = function () {
            app.files.push($(this).val());
        };

        app.doUpload = function () {
            alert('uploading ' + app.files.length.toString() + ' files...');
        };

        app.clear = function () {
            app.files = [];
            app.initOptions.fileCtrls.val('');
        };

        app.init = function (options) {
            app.initOptions = options;

            // wire up the events
            options.clearLink.click(app.clear);
            options.uploadBtn.click(app.doUpload);
            options.fileCtrls.change(app.addFile);
        };

        return app;

    }($));

    $(document).ready(function () {
        var i;

        // initialization call on the app:

        App.init({
            clearLink: $('#btnClear'),
            uploadBtn: $('#btnUpload'),
            fileCtrls: $("input[type='file']")
        });


        // QUnit test examples:

        test("Set File Input", function () {
            for(i = 0; i < App.initOptions.fileCtrls.length; i++) {
                App.initOptions.fileCtrls[i].value = 'C:/fakepath/document--add.png';
                $(App.initOptions.fileCtrls[i]).trigger('change');
            }
            equals(App.files.length, i, 'files array has ' + i.toString() + ' files');
        });

        test("Clear Files", function () {
            $('#btnClear').trigger('click');
            ok((App.files.length == 0), 'files array is empty');
            for(i = 0; i < App.initOptions.fileCtrls.length; i++)
            {
                equals(App.initOptions.fileCtrls[i].value, '', 'file input ' + i.toString() + " value is empty");
            }
        });
    });
    </script>
</body>

share|improve this answer
    
I'm not sure I understand how this refactored app is more testable than my initial version. In both cases the class is basically a singleton (so state is shared between tests unless the page is reset or we add a reset function to our app). This is the #1 problem I see with this codes 'testability'.You have made a few things additional things public in your version that were private in mine which could be nice. –  silent__thought Jun 29 '11 at 13:05
    
Just a small critique (or FYI if you were not aware of this). In your defined closure that builds the App object, you should be passing in jQuery instead of $ and then have the anonymous function parameter be your $. jQuery is the global object that jQuery creates. The $ is just an alias for that object and is also used in other frameworks. Passing in the jQuery object can isolate your snippet better. –  JamesEggers Jun 29 '11 at 15:01
add comment

//original code:

/* var MyPage = function($) { var _files , Dom = (function($){ return { clearLink: $("#btnClear") , uploadButton: $("#btnUpload") , filesControl: $("input[type=file]") }; }($)) */ This part can get really messy if you are monkeying the DOM like that.. can cause severe troubles to your browser

        //something like this will do the trick
        var the_Form= $('form:eq(0)'); //find our form

        var MyPage= jQuery(function($){
                var _files,
                         Dom= (function($){
                        return{
                        //rediscover the elements, I would have done it with a class and checked on hasClass('class') for better testing from the frontend part
                                    clearLink: the_Form.find('a#btnClear'),
                                    uploadButton: the_Form.find('input[type="submit"]#btnUpload'),
                                    filesControl: the_Form.find("input[type='file']")

... continue with your program.. I believe that you need to have a method to destroy the element if you don't reload the page or want to do some real time interaction with AJAX perhaps for file uploading.. as there might be some of those objects existing afterwards..

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.