Print out yarn install when yarn.lock file is present

[samdemaeyer]

As some might know, when you are using yarn and you remove /node_modules, you will have a comment when you try to run ember server

node_modules appears empty, you may need to run `npm install`

I fixed something similar in ember-cli-dependency-checker.
It's quite confusing for people who are not that familiar with the package managers.

As this is my first PR to ember-cli, I am guessing that there might be a better way for this fix. But at least the idea is out there now.

In this PR, I will print out

node_modules appears empty, you may need to run `yarn`

when there is a yarn.lock file present.

I know that yarn is not used by ember-cli by default yet, but it might be ok to check this as in the team I am working in, it got confusing for some people .. 😅

[rwjblue]

I'd prefer to check it here directly, and not add another method to project.

If however, we do keep the project method around, we should update other locations that search for yarn.lock to use it (example here).

[samdemaeyer]

@rwjblue, we only use it in /lib/commands/init.js:64 and lib/tasks/npm-task.js:45
I can take it out and check it directly if you like.
Than I just need another approach to test it, ATM, we overwrite the hasYarnLockFile function in the project class:

command.project.hasYarnLockFile = function() { return true; };

[samdemaeyer]

@rwjblue, in this commit I will check it directly. To test this behaviour, I am renaming the ember-cli project's actual yarn.lock file before the test runs and after the test, rewrite it back to the original name.
Could you advise me if this is a good approach, or might it be to risky? If so, what would you suggest for testing?

[Turbo87]

nice work!

[Turbo87]

can we make this yarn install to make it a bit easier to understand?

[samdemaeyer]

@Turbo87, sure, no prob.

[Turbo87]

this renameSync will not run if the assertion above fails. you could wrap it in a describe() block though:

describe('without yarn.lock file', function() {
  let originalYarnLockPath, dummyYarnLockPath;

  beforeEach(function() {
    originalYarnLockPath = path.join(command.project.root, 'yarn.lock');
    dummyYarnLockPath = path.join(command.project.root, 'foo.bar');
    fs.renameSync(originalYarnLockPath, dummyYarnLockPath);
  });

  afterEach(function() {
    fs.renameSync(dummyYarnLockPath, originalYarnLockPath);
  });

  it('runs GenerateFromBlueprint but with null nodeModulesPath', function() {
    command.project.hasDependencies = function() { return false; };

    return expect(command.validateAndRun(['controller', 'foo'])).to.be.rejected.then(reason => {
      expect(reason.message).to.eql('node_modules appears empty, you may need to run `npm install`');
    });
  });
});
``

[samdemaeyer]

Thanks @Turbo87, makes sense!
Question, would you typically destroy the global variables in the afterEach function?

afterEach(function() {
  fs.renameSync(dummyYarnLockPath, originalYarnLockPath);
  originalYarnLockPath = dummyYarnLockPath = null;
});

[Turbo87]

since they are reassigned in every beforeEach() block there is IMHO no need to reset them in afterEach()

[Turbo87]

@homu r+

[homu]

📌 Commit bc1241f has been approved by Turbo87

[homu]

⌛ Testing commit bc1241f with merge 2da9de5...

[homu]

☀️ Test successful - status