Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test coverage incorrectly reported for 'action' property in Iron Router #135

Open
jrgrafton opened this issue Feb 9, 2014 · 12 comments
Open

Comments

@jrgrafton
Copy link

http://d.pr/i/7z03

As you can see above the coverage for 'action' in routes.js is reported as missing even though it has been executed as part of the selenium acceptance tests.

May it be that Istanbul is unable to capture coverage for 'server' methods in Iron Router?

@samhatoum
Copy link
Member

Hey. You want to exclude any 3rd party software from the code coverage

@jrgrafton
Copy link
Author

Hey Sam,

Thanks for getting back to me. So I completely agree with the exclusion of third party libraries, although in this case since Iron Router is performing some non trivial 301 redirect it would be great to have it covered.

Is there any way to get Istanbul to cover these additional execution blocks? Maybe push the logic back into a file in /server/?

@samhatoum
Copy link
Member

My bad, I didn't really understand the issue. I see you have the file in /app/lib/router.js which means it's being covered. Are you sure you're hitting these lines with selenium? This is either the a bug in the coverage report not coming back in time, or that the code is not hitting it. Can you check with some console.logs and see if the selenium run really touches that part?

@jrgrafton
Copy link
Author

Hey Sam,

No worries, I'm pretty new to Meteor so just trying to get my head around exactly to expect myself!

I've just rerun the tests with logging. As you can see here the logs were definitely output during Selenium testing, however the coverage still reports them as not having been executed.

Could it be that Istanbul is simply unable to report coverage for the action function in Iron Router?

EDIT: Maybe this has something to do with the fact that the 301 server redirect is navigating context away from the page before Istanbul can post back code coverage?

@samhatoum
Copy link
Member

Quite possibly. The first thing to check is if the code is being instrumented, which I would imagine it is. Could you have a look at .../shortly/production/build/mirror_app/lib/router.js and see if the file has a global coverage object? If it does, it means your last statement is probably true, as the coverage would be being reported but is not being posted back when the redirect happens , which we'd have to patch/fix. But let's be sure first.

@jrgrafton
Copy link
Author

Looks like the global coverage object exists.

Seems like it could be quite a difficult fix though if the redirect happens inside an Iron Router server action - interested to hear how this could be resolved!

@samhatoum
Copy link
Member

Naaahh, this is JS and it's easy :)

We'd have to patch the iron-router method for the mirror app. So if this is the normal flow in the Iron Router server:

ironRouterServer.doRedirect(<the args>) {
    //do some stuff
}

We would wrap that method by doing this in our setup code:

oldDoRedirect = ironRouterServer.doRedirect;
ironRouterServer.doRedirect = function(<the args>) {
    // post back coverage here
    oldDoRedirect(<the args>);
}

This allows us to insert some steps into the normal flow and that's where we'd do the postback coverage before serving up the 301.

Fancy giving that a try?

@samhatoum
Copy link
Member

@cmather you may be mildly interested in this

@jrgrafton
Copy link
Author

Well I gave it a quick go, by copying the postCoverage function to the routes.js file I'd expect the first three lines of the action function to be picked up - yet it appears that it's still not registering.

@samhatoum
Copy link
Member

Interesting. Next thing to check is of the post back is being received. Perhaps you can look in the network inspector of chrome/Firefox?

@jrgrafton
Copy link
Author

Post back seems to be happening but coverage still isn't being captured, strangely enough even when I add it to the exclusion list it still reports no coverage

instrumentationExcludes: ['**/router.js', '**/packages/**', '**/3rd/**', 'fixture.js', 'fixture.coffee'],

@samhatoum
Copy link
Member

This is odd. Do you think you could replicate this in an isolated project? That would help me fix it

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

No branches or pull requests

2 participants