Saw this, looks pretty good! Here's some initial questions:

1. Can we add some beforeEach steps to the tests to make sure they all start in a standardized wishing directory, with the same contents in the test directory, etc?
2. I noticed that you unwrapped the callbacks in echo. Doesn't this mean that those test cases will report success before actually calling the callbacks (which is where the real test is)? Is there any way to prevent that from happening?
3. I saw that the tests were in es6. Will Ava take care of the translation? It may be worth it to write the tests in es 5 if it cuts down on deps.

@ariporad all tests should be working for sync, but we'll need to refactor plugin.js for async. Here's what I want to do (in mocha syntax, since that's what I'm familiar with):

describe('before doing plugin', () => {
  it('does not exist before being registered', () => {
    t.is(!shell.foo);
  });
});
describe('before doing plugin', () => {
  beforeEach(() => {
    plugin.register('foo', fooImplementation, {
      cmdOptions: {
        'f': 'flag',
      },
      wrapOutput: true,
      canReceivePipe: true,
    });
  });

  afterEach(() => {
    // do something to un-register the plugin
  });

  it('exists after registering', () => {
    t.is(shell.foo);
  });
});

I'm not sure what the equivalent of describe is in ava, but basically I need a way to make sure the plugin is not registered before some of the tests, and is registered before some other tests, so I need a different beforeEach() step depending on the test.

@nfischer: Wow... you did a lot!

Your questions, in order:

1. We could.
2. We'd need to use test.cb.
3. AVA automagically babels the tests, but not the source.
4. You'd need a seperate test file, there is no describe in AVA.

I'm going to be busy till tomorrow morning, but I'll try to review everything then.

@ariporad would we be able to serialize the tests within plugin.js (once everything else is parallel by default)? It seems like test.serial() would work for us, but I'm not sure if we only need to put it on the first test case or if we need to put it on all the test cases in that file.

@ariporad I'll leave the rest of this to you, since there shouldn't be much more. Ping me on here when it's ready for review 😄

@nfischer: Ok, this should be good now, it all passes lint. I don't have a way to test on windows, but otherwise should be good.

@ariporad the tests are failing because we used to skip some tests when running on Windows (chmod was notorious for that). If you check master branch, you'll see which test cases to skip. Probably the best way I can think of is:

test('title', t => {
  if (windows) return;
  // otherwise proceed as normal
});

@ariporad are we still working on this? If so, would you be able to rebase this off master?

@freitagbr (and any one else), feel free to help take this on. Tests are still very flaky when run in parallel mode.

Also, here's a running list of the files I've verified have not lost any test cases during the refactor:

• cat.js
• cd.js
• chmod.js
• common.js
• config.js
• cp.js
• dirs.js
• echo.js
• env.js
• exec.js
• find.js
• global.js
• grep.js
• head.js
• ln.js
• ls.js
• mkdir.js
• mv.js
• pipe.js
• plugin.js
• popd.js
• pushd.js
• pwd.js
• rm.js
• sed.js
• set.js
• shjs.js
• sort.js
• tail.js
• tempdir.js
• test.js
• toEnd.js
• to.js
• touch.js
• uniq.js
• which.js

I think I know why the tests are failing. Let's look at grep.js for example:

let TMP;

test.beforeEach(() => {
  TMP = utils.getTempDir();
  shell.config.silent = true;
  shell.cp('-r', 'resources', TMP);
});

Isn't the value of TMP going to be overwritten with every test? So one of the grep tests is looking to see if the TMP exists, but the value of TMP intended for that test has already been overwritten by the value of TMP for another test.

In other words, we can't have any globals in these async tests. That, or we keep a stack or something of all the tmp directories that get created.

Solution to the problem, do something like this:

test.beforeEach(t => {
  const tmp = t.context.tmp = utils.getTempDir();
  shell.config.silent = true;
  shell.cp('-r', 'resources', tmp);
});

test('example' t => {
    // t.context.tmp is the tmp dir for this test
});

@freitagbr ah, that looks perfect. That's just what I was looking for. I'll do the refactor tonight and push again

@nfischer I already wrote the code to test it. Can I submit a PR to this branch?

Even after fixing that bug, I still get an error:

> ava test/*.js

   451 passed
   1 failed

   1. tail › reading more lines than are in the file (no trailing newline)

  t.is(result.toString(), 'test2\ntest1')
              |
              "test1\ntest1"

      "test1/ntest1"
        Test.fn (tail.js:74:5)
        _combinedTickCallback (internal/process/next_tick.js:67:7)
        process._tickCallback (internal/process/next_tick.js:98:9)

@freitagbr whoops, didn't see your comment. Take a look at the commit I just uploaded here. If I missed anything, feel free to correct it

Alright. Since we're starting to get merge conflicts with other patches that are going through, and I'm tired of rebasing, I think we should start merging these files one at a time. Here's the proposed strategy to do this in a sane way:

• make a directory called ava-test/
• copy the resources to ava-test/resources/ (leave a copy where it is now, too)
• one by one, remove a file from test/ and put its ava equivalent in ava-test/
• modify npm test: instead of node scripts/run-tests, turn it into node scripts/run-tests && ava --serial ava-test/*.js

Once all tests are refactored like this, we just do rm -rf test/ && git mv ava-test test.

@freitagbr what are your thoughts on this plan?

I think that's a good plan. Just keep an eye community contributions, they might get confused as to where their tests should go.

Closing this, since we're almost done migrating all the tests