Skip to content
This repository has been archived by the owner on Aug 7, 2020. It is now read-only.

Saga doesn't generate correct coverage metrics #96

Open
JonCook opened this issue Jun 28, 2013 · 16 comments
Open

Saga doesn't generate correct coverage metrics #96

JonCook opened this issue Jun 28, 2013 · 16 comments

Comments

@JonCook
Copy link

JonCook commented Jun 28, 2013

So it is really great I can plug saga straight into mvn and I get a report! Can't believe it!! However, always a however. I have the following test and corresponding js. I know the test definitely covers most of the js file in question but saga doesn't seem to recognise this and reports very low coverage. I wondered what I'm doing wrong, maybe it is the way the widget is constructed? We don't use new so maybe saga won't work in this way? Probably the widget code can be improved but that isn't the point yet..

We are using mocha-phantomjs testrunner.

Some ideas?

Thanks very much
Jon

widget under test:

/*
    ToggleButton that filter your search in more options.
    On  - if you only want Direct flights only
    Off - if you don't only want Direct flights only

    Optional params:
        defautlStatus:Toggle button will start turned on if the value is equal to "on".
                       Othercase, even if not defined toggle button will start turned off.
                       Allowed values (on|off)
 */

$(function(){
    Odigeo.defineWidget('togglebutton', Odigeo.SimpleWidget.extend({
        instanceAttr: {
            options: {
                inputSelector: 'input[type="checkbox"]',
                containerSelector: 'toggleButton'
            },

            data:false
        },

        init: function(){
            this.bindToDom();
            this.attr.data = false;
        },

        //save DOM selectors to the widget cache
        cacheDomSelectors: function(){
            this.addSelectorToCache('inputSelector',                this.attr.$widget.find(this.attr.options.inputSelector));
            this.addSelectorToCache('containerSelector',            this.attr.$widget.find('.' + this.attr.options.containerSelector));
            this.addSelectorToCache('toggle_container_selector',    this.attr.$widget.find('.' + this.attr.options.toggle_container_selector));
        },

        /**
        bind handlars to DOM events here
        */
        bindToDom: function(){
            if(!this.attr.isDomBinded) {
                this.attr.$cachedSelectors['containerSelector']
                .bind(Odigeo.Events.swipeLeft + " " + Odigeo.Events.swipeRight + " " + Odigeo.Events.tap, _.bind(this._handlerDispatcher, this, {widgetID: this.id,
                    handler: _.bind(this.swipeHandler, this)}));

                this.attr.isDomBinded = true;
            }
        },

        unbindFromDom: function(){
            if(this.attr.isDomBinded) {
                this.attr.$cachedSelectors['containerSelector']
                .unbind(Odigeo.Events.swipeLeft + " " + Odigeo.Events.swipeRight + " " + Odigeo.Events.tap);

                this.attr.isDomBinded = false;
            }
        },

        setDefault: function() {
            this.attr.data = (this.attr.options.defaultStatus === "on");
            if(!this.attr.options.silent){
                this._triggerComplete();
            }
        },

        /**
        private methods to access interface
        @private
         */
        runCommand: function(func, params) {
            if(func === 'setDefault') {this.setDefault(params); return;}
            else if(func === 'setValue') { this.switchState(params.value, params.silent); return;}
        },

        switchState:function(state, silent){
            var checkboxState = '';
            silent = silent || this.attr.options.silent;
            if(typeof state === 'undefined'){
                this.attr.data = !this.attr.data;
            }else{
                // ********** need to check false (string) - > false (bool)
                this.attr.data = (state === 'true') ? true : false;
            }
            if(this.attr.data){
                this.attr.$cachedSelectors["containerSelector"].addClass('active');
                checkboxState = 'checked';
            }else{
                this.attr.$cachedSelectors["containerSelector"].removeClass('active');
            }

            this.attr.$cachedSelectors["inputSelector"].prop('checked', checkboxState);
            if(!silent){
                this._triggerComplete();
                this._triggerFocus();
            }
        },

        swipeHandler: function(event) {
            if(event){
                event.stopPropagation();
            }

            if((event.type == Odigeo.Events.swipeRight && this.attr.data !== true)
            || (event.type == Odigeo.Events.swipeLeft && this.attr.data == true)
            || event.type ==Odigeo.Events.tap){
               this.switchState();
            }
        },

        /* * * * * Widget Triggers * * * * */

        _triggerFocus: function() {
            if(!this.attr.isDisabled){
                this.bindObject.trigger('focus');
            }
        },

        _triggerComplete: function(widgetName) {
            var data = this.attr.data,
            schema = {};

            //after autocomplete appears blur event trigger without any data
            if(typeof data !== "undefined") {
                schema = data;
            }

            if(schema){
                data = "true";
            } else {
                data = "false";
            }

            this.bindObject.trigger('complete', {
                'type': 'success',
                'data': data
            });
        }

    }));
});

Test

var toggleButton = null;

beforeEach(function () {
    Odigeo.constructSimpleWidget("test-toggle-button", "togglebutton");
    toggleButton = Odigeo.getWidgetByID('test-toggle-button');
});

describe("Toggle button unit tests", function () {

    it("should be a simple widget", function () {
        expect(toggleButton.isSimpleWidget).to.be(true);
    });

    it("should be of type togglebutton", function() {
        expect(toggleButton.type).to.eql('togglebutton');
    });

    it("should have the same id it was constructed with", function() {
        expect(toggleButton.id).to.eql('test-toggle-button');
    });

    it("should be binded to DOM after initialisation", function() {
        expect(toggleButton.attr.isDomBinded).to.be(true);
    });

    it("should hava false data after initialiation", function () {
        expect(toggleButton.attr.data).to.be(false);
    });

    it("should add active class to container when setValue is true", function() {
        toggleButton.attr.options.defaultStatus = "off"

        var toggleContainer = toggleButton.attr.$widget.find('.'+toggleButton.attr.options.containerSelector);
        expect(toggleContainer[0].className).not.to.contain('active');

        toggleButton.runCommand('setValue', {value:'true', silent:true});
        expect(toggleContainer[0].className).to.contain('active');
    });

    it("should add checked property to inputselector when setValue is true", function() {
        toggleButton.attr.options.defaultStatus = "off"

        var inputSelector = toggleButton.attr.$widget.find(toggleButton.attr.options.inputSelector);
        expect(inputSelector[0].checked).to.be(false);

        toggleButton.runCommand('setValue', {value:'true', silent:true});
        expect(inputSelector[0].checked).to.be(true);
    });

    it("should call both triggers when silent is false", function() {
        sinon.spy(toggleButton, "_triggerComplete");
        sinon.spy(toggleButton, "_triggerFocus");

        toggleButton.runCommand('setValue', {value:'true', silent:false});
        expect(toggleButton._triggerComplete.calledOnce).to.be(true);
        expect(toggleButton._triggerFocus.calledOnce).to.be(true);
        toggleButton._triggerComplete.restore();
        toggleButton._triggerFocus.restore();
    });

    it("should trigger switch state on tap event", function() {
        sinon.spy(toggleButton, "switchState");

        toggleButton.attr.$cachedSelectors.containerSelector.trigger(Odigeo.Events.tap);
        expect(toggleButton.switchState.calledOnce).to.be(true);
        toggleButton.switchState.restore();
    });

    it("should not trigger switch state on swipe left event with false data", function() {
        sinon.spy(toggleButton, "switchState");

        toggleButton.attr.$cachedSelectors.containerSelector.trigger(Odigeo.Events.swipeLeft);
        expect(toggleButton.switchState.calledOnce).to.be(false);
        toggleButton.switchState.restore();
    });

    it("should trigger switch state on swipe left event with data", function() {
        toggleButton.attr.data = true;

        sinon.spy(toggleButton, "switchState");

        toggleButton.attr.$cachedSelectors.containerSelector.trigger(Odigeo.Events.swipeLeft);
        expect(toggleButton.switchState.calledOnce).to.be(true);
        toggleButton.switchState.restore();
    });

    it("should trigger switch state on swipe right event", function() {
        sinon.spy(toggleButton, "switchState");

        toggleButton.attr.$cachedSelectors.containerSelector.trigger(Odigeo.Events.swipeRight);
        expect(toggleButton.switchState.calledOnce).to.be(true);
        toggleButton.switchState.restore();
    });

    it("should not trigger switch state on swipe right event when data is true", function() {
        toggleButton.attr.data = true;
        sinon.spy(toggleButton, "switchState");

        toggleButton.attr.$cachedSelectors.containerSelector.trigger(Odigeo.Events.swipeRight);
        expect(toggleButton.switchState.calledOnce).to.be(false);
        toggleButton.switchState.restore();
    });

    it("should call stop propagation on event when tap event triggered", function() {
        var event = jQuery.Event( "tap" )
        var mock = sinon.mock(event);

        mock.expects('stopPropagation').once();
        toggleButton.attr.$cachedSelectors.containerSelector.trigger(event);
        mock.verify();
    });

});

afterEach(function () {
    toggleButton.runCommand('setValue', {value:'false', silent:true});
    toggleButton = null;
});
@JonCook
Copy link
Author

JonCook commented Jun 28, 2013

Maybe it is to do with this.

11:33:07.196 [pool-2-thread-3] WARN  com.github.timurstrekalov.saga.core.htmlunit.QuietJavaScriptErrorListener - Error loading script referenced on page file:/home/password/projects/frontend-html5-fe-refactor-jonathan-cook/layers/front-layer/target/test-classes/com/odigeo/front/javascript/tests/test.html. Script URL: file:/home/password/projects/framework/src/main/resources/META-INF/resources/static-content/_versionedByRelease_/js/odigeo.ui.js, message: Unable to download JavaScript from 'file:/home/password/projects/framework/src/main/resources/META-INF/resources/static-content/_versionedByRelease_/js/odigeo.ui.js' (status 404).

I bumbed the version to 1.4.2 as saw some phantom related fixes but no luck??

@mprins
Copy link
Contributor

mprins commented Jun 28, 2013

your pom snippet may be more useful here as the error indicates it's failing to load a script

@JonCook
Copy link
Author

JonCook commented Jun 28, 2013

pom snippet. if it can't find the scripts why does it generate a report? I saw some commit related to phantomjs and local http servers?

<plugin>
                <groupId>com.github.timurstrekalov</groupId>
                <artifactId>saga-maven-plugin</artifactId>
                <version>1.4.2</version>
                <executions>
                    <execution>
                        <phase>test</phase>
                        <goals>
                            <goal>coverage</goal>
                        </goals>
                    </execution>
                </executions>
                <configuration>
                    <baseDir>${basedir}</baseDir>
                    <includes>
                        **/*test*.html
                    </includes>
                    <noInstrumentPatterns>
                        <pattern>.+/src/test/resources/com/odigeo/front/javascript/tests/tools/expect\.js</pattern>
                        <pattern>.+/target/test-classes/com/odigeo/front/javascript/tests/tools/expect\.js</pattern>
                        <pattern>.+/src/test/resources/com/odigeo/front/javascript/tests/tools/sinon\.js</pattern>
                        <pattern>.+/target/test-classes/com/odigeo/front/javascript/tests/tools/sinon\.js</pattern>
                        <pattern>.+/src/test/resources/com/odigeo/front/javascript/tests/tools/mocha\.js</pattern>
                        <pattern>.+/target/test-classes/com/odigeo/front/javascript/tests/tools/mocha\.js</pattern>
                        <pattern>.+/src/test/resources/com/odigeo/front/javascript/tests/widgets/hotelsandcarbuttons\.js</pattern>
                        <pattern>.+/src/test/resources/com/odigeo/front/javascript/tests/framework/odigeo\.js</pattern>
                        <pattern>.+/target/test-classes/com/odigeo/front/javascript/tests/framework/odigeo.ui\.js</pattern>
                        <pattern>.+/src/test/resources/com/odigeo/front/javascript/tests/framework/odigeo.ui\.js</pattern>
                        <pattern>.+/src/test/resources/com/odigeo/front/javascript/tests/widgets/togglebutton\.js</pattern>
                        <pattern>.+/target/test-classes/com/odigeo/front/javascript/tests/widgets/togglebutton\.js</pattern>
                        <pattern>.+/target/test-classes/com/odigeo/front/javascript/tests/framework/odigeo\.js</pattern>
                        <pattern>.+/framework/src/main/resources/META-INF/resources/static-content/thirdParty/underscore\.js</pattern>
                        <pattern>.+/framework/src/main/resources/META-INF/resources/static-content/_versionedByRelease_/js/helper\.js</pattern>
                        <pattern>.+/framework/src/main/resources/META-INF/resources/static-content/thirdParty/jquery-1.8.2\.js</pattern>
                        <pattern>.+/target/test-classes/com/odigeo/front/javascript/tests/widgets/hotelsandcarbuttons\.js</pattern>
                    </noInstrumentPatterns>
                    <outputDir>${basedir}/target/surefire-reports/javascript-coverage-reports/</outputDir>
                </configuration>
            </plugin>

@mprins
Copy link
Contributor

mprins commented Jun 28, 2013

I have no experience using mocha/sinon, i've only used jasmine.
The local server usage is described here: http://searls.github.io/jasmine-maven-plugin/code-coverage.html but is jasmine specific afaik

@JonCook
Copy link
Author

JonCook commented Jun 28, 2013

Ok thanks, but I'm not using a local server. We are reading from the file system.

@JonCook
Copy link
Author

JonCook commented Jun 28, 2013

I fixed this error but still no change in the metrics and a different ERROR, do these stop the metrics being generated.

15:50:54.620 [pool-2-thread-1] ERROR com.github.timurstrekalov.saga.core.htmlunit.QuietJavaScriptErrorListener - Script exception on page file:/home/password/projects/frontend-html5-fe-refactor-jonathan-cook/layers/front-layer/src/test/resources/com/odigeo/front/javascript/tests/test.html, message: TypeError: Cannot read property "width" from undefined (file:/home/password/projects/frontend-html5-fe-refactor-jonathan-cook/layers/front-layer/src/test/resources/com/odigeo/front/javascript/tests/tools/mocha.js#369), failing line: <no source>

@JonCook
Copy link
Author

JonCook commented Jun 28, 2013

Looks like related to #89 and #86 so can probably conclude it doesn't work with mocha-phantomjs

@timurstrekalov
Copy link
Owner

The very raw version of possibly working PhantomJS support is in the snapshot version, not yet released, so 1.4.2 wouldn't have it either. And, sadly, I don't have any experience with mocha either, so if someone does - jump in.

@JonCook
Copy link
Author

JonCook commented Jul 19, 2013

I have experience with mocha-phantomjs but what would I need to do to start experimenting or extending saga? I would be happy to contribute :)

@timurstrekalov
Copy link
Owner

@JonCook that would be great! Most of the things you'd want are under saga-core (everything else is concrete implementations for various build systems and what not), you can take a look at the DefaultCoverageGenerator class where it all begins. Sadly, there's scant little documentation on it (almost none, I'd say), but I'd like to think that it isn't too big of a mess to be able to understand it without the docs 😄

The most interesting thing in terms of this issue is how to instrument (and how to use the gathered stats, of course). The instrumentation itself, i.e. once you've already got your hands on the source code of a script, should work by now (that's your HtmlUnitBasedScriptInstrumenter class), so that shouldn't give you any big trouble. Take a look at the GenericInstrumentingBrowser class – what it does in order to be able to instrument the code is start up a proxy server and use it when pulling the scripts – this way, some degree of instrumentation is actually possible when using a generic WebDriver (that's running in an actual browser whose JS engine we don't have access to).

Last but not least, it uses Ghost Driver in order to talk to PhantomJS. So, kickstarts the driver, starts up a proxy server, sets it to be used by the driver, fires up the test and then collects the data.

Let me know if you've got any questions 😄

@JonCook
Copy link
Author

JonCook commented Jul 22, 2013

Thanks. One quick question, I noticed this test in DefaultCoverageGeneratorIT

@test
@ignore
public void test_instrumentAndGenerateReports_with_phantomjs() throws Exception {

Seems to be ignored and if I remove it fails, this is probably the test I'm interested in because it mentions phantom.

THanks

@timurstrekalov
Copy link
Owner

@JonCook AFAIR it's only ignored because it mentions a full path to the phantomjs binary, and I didn't have the time to make it better, but apart from that, the test is supposed to work, I think

@JonCook
Copy link
Author

JonCook commented Aug 20, 2013

Hi,

I've had some time to play with this and appears to work with the existing phantom functionality. I created a new java unit tests and js files and it works. Would you like me to submit my test files?

I do have another question, looks like 1.5 is stable now, do I need to put something in my pom.xml to see to use a phantomjs binary etc?

Thanks
Jon

@timurstrekalov
Copy link
Owner

@JonCook absolutely, tests are very welcome!

To use PhantomJS you only need to put the web driver class name (see this comment for details).

To specify the path to the phantomjs binary, you can use the webDriverCapabilities element and configure the "phantomjs.binary.path" property to point to it (unless it's already on the PATH) - the idea is that it's exactly the same as in Jasmine.

Sorry there are no docs about this yet 😿

@JonCook
Copy link
Author

JonCook commented Aug 21, 2013

@timurstrekalov - Thanks
Sorry for the stupid questions, so I have it working in a unit test but if I add this for example to my pom.xml

    <baseDir>http://10.248.40.101:8234/${basedir}/src</baseDir>
    <webDriverClassName>org.openqa.selenium.phantomjs.PhantomJSDriver</webDriverClassName>

I get a connect refused exception every time, looks like ghostdriver allocates a different port each time it starts up so how can I put the correct port?

@timurstrekalov
Copy link
Owner

Are you sure the exception is coming from the PhantomJS port rather than Saga trying to connect to that URL over there? Would be nice to see the stacktrace...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants