jquery-mockjax is enabled in development as well as test environment

[begedin]

Looking at the addon's included hook, this is what we have:

included: function(app) {
  this._super.included(app);

  if (app.tests) {
    app.import(app.bowerDirectory + '/jquery-mockjax/dist/jquery.mockjax.js');
  }
}

My best guess, the intended behavior here is for mockjax to load only in a test environment. However, app.tests is true in the development environment as well.

This on its own doesn't cause any trouble, but in one specific case I'm aware off, and probably other similar cases, it completely breaks ajax behavior.

The specific case I'm talking about is when sentry with raven.js is used in the same project.

raven.js does its own wrapping of ajax requests for the purpose of error logging. It does this in a similar fashion as mockjax, but the unfortunate consequence is that the mockjax wrapper is executed after the sentry wrapper and at some point, the ajax URL is blanked out. Since it's blanked out, the default behavior is to use the current browser URL.

For example, in our ember app, we have a route under the url /signup. When submitting the form there, an ajax POST request should go to api/users, but due to using both sentry and ember-data-factory-guy (which is loading mockjax), the URL the request is posted to ends up being localhost/signup, which doesn't exist at all.

In my opinion, the underlying issue is incompatibility between raven.js and jquery-mockjax, so I intend to submit issues to both repos, but in the meantime, it also seems that checking app.tests to determine if jquery-mockjax should be loaded with ember-data-factory-guy is a bug all on it's own.

My suggested solution is to instead check app.env:

included: function(app) {
  this._super.included(app);

  if (app.env === 'test') {
    app.import(app.bowerDirectory + '/jquery-mockjax/dist/jquery.mockjax.js');
  }
}

I will create a PR which adds this correction. If I'm wrong about it and app.tests is in fact the value the addon shoould be checking, then I stand corrected.

• Related PR: Changed how to determine if mockjax should be loaded #125
• Issue in mockjax repo - Conflict with raven.js jakerella/jquery-mockjax#267

[begedin]

To give an update, the mockjax team has agreed this is a bug in their library and they intend to resolve it on 2.1.0. On this side, I think this can be considered resolved with the merge of #125.

[danielspaniel]

@begedin #125 brings up the reason I had this originally done it that way .. to run test in qunit in development mode.