Cleanup unit tests that wait for a promise

[jasonsanjose]

We have older async unit tests that follow this pattern when waiting for a promise

var done, err;

runs(function () {
  asyncCall().done(function () {
    done = true;
  }).fail(function () {
    err = true;
  });
});
waitsFor(function () { return done && !err; });

We can reduce this to...

runs(function () {
  waitsForDone(asyncCall());
});

...assuming that we're not inspecting the values from the done or fail callbacks.

[peterflynn]

And fwiw, even if we are inspecting the callback values, it can still be simplified:

runs(function () {
  var promise = asyncCall();
  promise.done(function (result) {
    expect(result.foo).toBe(bar);
  });
  waitsForDone(promise);
});

[thefirstofthe300]

I would like to take this on.

Before I start, is the only unit test suite that needs to be fixed Async-test.js or are there others?

[thefirstofthe300]

I found this in CSS-Utils-test.js:

runs(function () {
    FileUtils.readAsText(fileEntry)
        .done(function (text) {
            spec.fileContent = text;
        });
});

waitsFor(function () { return (spec.fileContent !== null); }, 1000);

Is this the scenario you are talking about @jasonsanjose @peterflynn?

[jasonsanjose]

@DaBungalow yes

[TomMalbran]

WorkingSetView-test.js has some too. Check @jasonsanjose comment here #4635.

[thefirstofthe300]

As I go through these tests, I am noticing some JSLint errors. Should I go ahead and fix them? Or is that best saved for another issue?

[TomMalbran]

There are no errors on any of the files in spec. Are you running with the JSLint module updated? There were a few JSLint error when we switched to a module, but those should be fixed after the module update.

But, yes, fix any JSLint error you see, but make sure that the module is updated first.

[thefirstofthe300]

I apparently just got it updated because all errors disappeared after
some fiddling with submodules.

On 8/10/13, Tomás Malbrán notifications@github.com wrote:

There are no errors on any of the files in spec. Are you running with the
JSLint module updated? There were a few JSLint error when we switched to a
module, but those should be fixed after the module update.

But, yes, fix any JSLint error you see, but make sure that the module is
updated first.

---

Reply to this email directly or view it on GitHub:
#4638 (comment)

Danny Seymour
dannyseeless@gmail.com

[TomMalbran]

I thought so. When I switched JSLint to a submodule I got a older version by mistake. I thought it was ok until I started seeing more errors, so I made a PR to update it to the version that we had before, to fix those errors.

[bmax]

Hi,

Is there a PR for this issue or should I go ahead and start on this?

[agusramos85]

Working on it.

[samlandfried]

I'll refactor some tests! 👍

[dhuang612]

Hi,

Is this available to work on?