Async/await refactoring for tests/io folder - test.directory.js

[hameleonka]

@rpl in this PR I've worked on test.directory.js in tests/io folder. Please check it out, waiting for your review.

[rpl]

Nit, there is no need to declare it as a function if we are not going to reuse it,
we could just store the promise:

const onceReadString = new Promise((resolve, reject) => {
  ...
});

const content = await onceReadString;
...

or we could move the function outside of the test function and pass the readStream as a parameter:

function readStringFromStream(readStream) {
  return new Promise((resolve, reject) => {
    ...
  });
}

describe('...', () => {
  it('...', async () => {
    ...
    const content = await readStringFromStream(readStream);
    ...
  });
});

[hameleonka]

@rpl I decided to go with second option because readStringFromStream(readStream) is used in 2 tests

[EnTeQuAk]

Sorry for the hiccup and weird test breakage, #1749 just got merged and once you rebase your branch to master tests should be working fine again :)

[rpl]

@hameleonka follows some additional comments related some final changes on this PR (just some code improvements that we can make before leaving this test file and move to the next two from the same dir), I'm pretty sure that is is going to be the last round of changes on this PR.

[rpl]

it looks that jest with a version >= 20 supports a expect(...).rejects assertion that we could use in out tests instead of the try { ... } catch (...) { ... }:

• https://facebook.github.io/jest/docs/en/expect.html#rejects

e.g. this one would become something like:

    await expect(
      myDirectory.getPath('whatever')
    ).rejects.toThrow('"whatever" does not exist in this dir.');

and as a plus jest produce a very nice failure message when the rejected promise doesn't match the expected error message (or if it doesn't fail as expected).

There are a number of try { ... } catch (...) { ... } in this file that we can make shorter and cleaner applying this approach, and it could be worth given that we are already changing this test file.

[rpl]

This helpers seems to be useful enough to be something exported by the helpers.js module (from the tests/ directory), I would not be surprised if it could be helpful in the xpi and crx test files as well.

[hameleonka]

@rpl thanks for the very helpful Jest link. Now I have a couple of suggestions about using Jest methods. For example, maybe here we can replace lines 106, 107 with
await expect(readStringFromStream(readStream).resolves.toBe('123\n');

[rpl]

@hameleonka 👍 sure! go for it!

[hameleonka]

@rpl I've changed some Jest methods in this file

[hameleonka]

@rpl same here
await expect(myDirectory.getFileAsString('dir2/dir3/file3.txt').resolves.toBe('123\n');

[rpl]

@hameleonka 👍