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

str is undefined when running the HTML function #2211

Closed
ubirajaramneto opened this issue Apr 15, 2016 · 6 comments
Closed

str is undefined when running the HTML function #2211

ubirajaramneto opened this issue Apr 15, 2016 · 6 comments

Comments

@ubirajaramneto
Copy link

ubirajaramneto commented Apr 15, 2016

Hey there!

I was testing my angular app in the browser when I was receiving a Type Error str is undefined.
After digging into the source code and the call stack, i found out that the calling function to the clean function was passing the object test.body, which was undefined, and that was causing the error.

I managed to put a sanity check to see if the str argument was undefined or not, and then the code followed on without producing the error but still i was not sure what the behavior would be, so i reverted my changes and decided to open an issue here in order to have it fixed and expose this bug.

STACKTRACE:
[39]</</exports.clean()
mocha.js:5936
HTML/<()
mocha.js:2518
[3]</EventEmitter.prototype.emit()
mocha.js:200
HTML/<()
mocha.js:2451
[3]</EventEmitter.prototype.emit()
mocha.js:200
[36]</</Runner.prototype.fail()
mocha.js:4564
[36]</</Runner.prototype.failHook()
mocha.js:4593
next/<()
mocha.js:4638
done()
mocha.js:4251
[35]</</Runnable.prototype.run()
mocha.js:4286
next()
mocha.js:4629
[36]</</Runner.prototype.hook/<()
mocha.js:4651
timeslice()
mocha.js:12619

Calling function passing undefined parameter:

// line 2518
 var pre = fragment('<pre><code>%e</code></pre>', utils.clean(test.body)); <--- test.body is undefined
      el.appendChild(pre);
      pre.style.display = 'none';

Function throwing the Type Error

// line 5935
exports.clean = function(str) {
  str = str <--- this guy is undefined
    .replace(/\r\n?|[\n\u2028\u2029]/g, '\n').replace(/^\uFEFF/, '')
    .replace(/^function *\(.*\)\s*\{|\(.*\) *=> *\{?/, '')
    .replace(/\s+\}$/, '');

  var spaces = str.match(/^\n?( *)/)[1].length;
  var tabs = str.match(/^\n?(\t*)/)[1].length;
  var re = new RegExp('^\n?' + (tabs ? '\t' : ' ') + '{' + (tabs ? tabs : spaces) + '}', 'gm');

  str = str.replace(re, '');

  return exports.trim(str);
};

Mocha Version 2.4.5
The distribution i am using is the Bower Component

@ubirajaramneto ubirajaramneto changed the title str is undefined when running the HTML function on line str is undefined when running the HTML function Apr 15, 2016
@ScottFreeCode
Copy link
Contributor

Do any of your tests run before it happens? If so, can you isolate what about a test triggers this?

I run into it sometimes when I have tests that are not marked as pending but are in a suite that is marked as pending. Since the isPending fixes hadn't made it in as of the last release, Mocha gets an error trying to treat the test as failed rather than pending, and that first error in turn somehow ends up causing a hook that is not a test with a body to go through the reporter and hit this issue. I am curious whether that's the same situation you're encountering or if there are other ways it can occur.

(The really weird thing is I don't always get any errors with that pending suite with non-pending tests. I will have to see if I can find out what makes the difference there.)

If it's the same immediate cause as my instance, then the next release (or, if you're up for keeping a local fork, an update of the built mocha.js file using the current master source) should get rid of it; but in either case it may be helpful to figure out whether it's expected that internal errors in Mocha will be reported and if they should be then what changes need to happen to ensure the reporter doesn't blow up like this on internal errors due to them not being from tests.

@ubirajaramneto
Copy link
Author

ubirajaramneto commented Apr 22, 2016

It is a failing test that triggers this, all of the passing tests were correct and was not triggering this issue.

The suite I was using had only one test suite with 3 tests in it inside a describe block.

I am not sure but after I put a check on the provoking function it worked fine, but since the codebase was large and I was not familiar with the code itself, I rolled back my changes and changed the reporter to the console, using the 'spec' option for the reporter.

I also did not marked any test as pending, nor did I use this resource at all.

And yes all tests runs, I followed the code execution and it happened after the tests ran and it was generating the report, if I am not mistaken you can see it in the stacktrace I put on the issue description.

@ScottFreeCode
Copy link
Contributor

Interesting; so, whatever's up seems to be almost completely different in our two cases...

I think the reporter actually outputs the result from each test after it runs, individually, rather than waiting for all of them to run; but if you saw all your tests in the output, then clearly they all must have run... Here's another thought -- do you have an after function, either on this suite or globally? If so, then an issue occurring after all tests run might be originating in that function; if not, I'm all out of ideas for the moment.

Well, all out of ideas other than that I'm going to try and deliberately create error situations in things other than tests and see if I can track down how Mocha gets to trying to report an error that has no test body.

@ScottFreeCode
Copy link
Contributor

Looks like #2115 fixes this, so we're just waiting on #2079 so we can get a new release.

I ran some tests and was able to determine that:

  • An error thrown from an after/afterEach hook will cause this issue.
  • In master such errors are properly reported instead.
  • In master the pending suite with non-pending tests doesn't cause issues anymore.
  • If I take master and revert the pending fix at the line that was causing my issue, but keep the rest of the changes including Fix hook error in browser reporter by moving body prop from test to runnable #2115, the internal error from the pending issue is reported, via uncaught exceptions handler, with an accurate stack location, no source code, and -- the only thing still incorrect -- "before all hook" as the place the error is alleged to have occurred (in fact, it occurs handling the result of a test, so I'm not sure why it lists the name of the previous hook, although I'm going to follow the error around a bit and see if I can find out).

So, basically, as far as I can tell #2115 prevents trouble with failure reports from exceptions outside tests, regardless of where and how the failure originated or which reporter is being used. We just need browser-testing in the continuous integration so the devs can be convinced to release another version.

@ubirajaramneto
Copy link
Author

Thanks a lot for the insight.

Also I would like to add the following, I am not sure if it makes a difference.

But I was testing an Angular.js application and I was using ng-mock for dependency injection of controllers and angular modules into the test suite.

Again I am not sure if this is something important, but maybe someone can relate to some other issue and maybe add it to their conclusions.

@danielstjules
Copy link
Contributor

Sorry :( #2115 fixed this and will go out with the next release!

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

No branches or pull requests

3 participants