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

bridge.js makes excess processing which results to OutOfMemory #44

Closed
evil-shrike opened this issue Aug 23, 2013 · 2 comments
Closed

bridge.js makes excess processing which results to OutOfMemory #44

evil-shrike opened this issue Aug 23, 2013 · 2 comments

Comments

@evil-shrike
Copy link

The plugin injects bridge.js script into PhantomJS for tests execution (here is it: https://github.com/gruntjs/grunt-contrib-qunit/blob/master/phantomjs/bridge.js)

There is such code there:

  QUnit.log(function(obj) {
    // What is this I don’t even
    if (obj.message === '[object Object], undefined:undefined') { return; }
    // Parse some stuff before sending it.
    var actual = QUnit.jsDump.parse(obj.actual);
    var expected = QUnit.jsDump.parse(obj.expected);
    // Send it.
    sendMessage('qunit.log', obj.result, actual, expected, obj.message, obj.source);
  });

I think it's a bad idea to parse (via QUnit.jsDump.parse) all objects being passed into QUnit's methods (like equal).

In my case this code results OutOfMemory due to complexity and recursion in passed objects.

@graue
Copy link

graue commented Mar 13, 2014

We've encountered a problem with this at work too. Not an out-of-memory, but severe slowness. Tests that take ~150ms to run in Chrome are taking ~23sec to run using grunt-contrib-qunit.

grunt-contrib-qunit only uses the arguments if the test failed, so this problem can be mostly solved, with no change in functionality, by calling jsDump.parse only in case of a test failure. (That's how QUnit itself works.)

I've forked grunt-contrib-qunit and added a fix based on 0.4.0. If I get around to it, I'll reapply my fix to master and submit a pull request. Otherwise, the fix is trivial and can be easily applied by anyone.

@Arkni
Copy link
Member

Arkni commented Feb 19, 2016

Resolved in #61

@Arkni Arkni closed this as completed Feb 19, 2016
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