backburner.js icon indicating copy to clipboard operation
backburner.js copied to clipboard

Missing test cases.

Open rwjblue opened this issue 7 years ago • 2 comments

I did a quick skim of our test coverage and noticed a few holes where we need some coverage:

  • bb.defer is untested (yes, it is deprecated, but it should still have a test given that it is public API)
  • Error conditions in bb.on and bb.off are not tested (passing no eventName, or passing no callback)
  • end called without begin error thrown in bb.end() is not tested
  • invoking bb.later() e.g. with no arguments. This should be an error IMHO, but currently just returns early
  • bb.ensureInstance() is public API that has no tests
  • bb.join(function() { }) (no target or args) when within a run-loop (e.g. when we are actually joining an existing loop) has no tests
  • bb.join(function() { }, someArg) (no target with args) when within a run-loop (e.g. when we are actually joining an existing loop) has no tests
  • } else { end = middle; } in binary-search has no tests
  • Queue.prototype.flush with a custom before / after has no tests

rwjblue avatar Jan 22 '18 14:01 rwjblue

I hope this is an OK place to put this, but perhaps an additional test might be needed for bb.join with no method:

// This is a stack trace from an Ember app where an async component action is being run, and the route is navigated away to another page during that time. I'm pretty sure there is no problem with the app code.

Uncaught (in promise) TypeError: Cannot read property 'apply' of undefined
    at Backburner._join (backburner.js:995)
    at Backburner.join (backburner.js:760)
    at Function.join (index.js:168)
    at Proxy.routeAction (route-action.js:53)
    at invokeAction (invoke-action.js:13)
    at runSearch (search.js:98)
    at SelectBox.search (index.js:327)
    at index.js:249
// backburner.js

_join(target, method, args) {
  if (this.currentInstance === null) {
    return this._run(target, method, args);
  }

  if (target === undefined && args === undefined) {
    return method();
  } else {
    return method.apply(target, args);  // `method` is undefined
  }
}

amk221 avatar Feb 24 '21 10:02 amk221

Ya, sounds good thanks @amk221!

rwjblue avatar Feb 26 '21 14:02 rwjblue