feat(ngcc): pause async processing if another process has the lockfile

[petebacondarwin]

PR Checklist

Please check if your PR fulfills the following requirements:

• The commit message follows our guidelines: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

• Bugfix
• Feature
• Code style update (formatting, local variables)
• Refactoring (no functional changes, no api changes)
• Build related changes
• CI related changes
• Documentation content changes
• angular.io application / infrastructure changes
• Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

• Yes
• No

Other information

[gkalpak]

LGTM, except I think we should reset the waiting period when the pid in the lockfile changes.
Really excited to incorporate this into the CLI workflow \o/

[ngbot]

I see that you just added the PR action: merge label, but the following checks are still failing:
     status "pullapprove" is failing
     forbidden labels detected: PR action: review
     status "google3" is pending
     3 pending code reviews

If you want your PR to be merged, it has to pass all the CI checks.

If you can't get the PR to a green state due to flakes or broken master, please try rebasing to master and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help.

[gkalpak]

This looks like a potential source of flakes 😁
Could we use fakeAsync() and tick() to make this test deterministic?

[petebacondarwin]

Yeah, I was a little bit worried about this. We cannot use the Angular fakeAsync stuff, since that is driven via Zone.js and we definitely not going to bring that in here just for testing...

I tried to make the timeouts as reliable as possible so that it would be extremely unlikely for it to flake:

• having proven that the class has already checked the lockfile (and read its value) we are immediately writing the new one synchronously so there is no chance for the setTimeout inside LockFile to be triggered.
• we then set a timeout for 250ms before the next check but the class has been configured to check the contents of the lockfile every 100ms. So there should be at least 2 opportunities for a setTimeout to be triggered before this testing one is triggered.
• The class is setup not to fail until 10 attempts have occurred, which is a full second, so there is very little chance that the 250ms timeout is not triggered before we run out of attempts.

Do you have any better ideas?

[gkalpak]

Fair enough. I am fine leaving this as is and address it in the future if we start seeing flakes.

For future reference, one way to make this deterministic would be using Jasmine's Clock. There is one twist, because we need to also flush microtasks after each retryDelay ms, so it would be something like:

const clock = jasmine.clock();
const moveToNextIteration = () => {
  clock().tick(100);     // Or whatever `retryDelay` we have specified.
  return Promise.resolve();  // To wait for microtask queue to be flushed.
};

clock.withMock(async () => {
  const rawFs = getFileSystem();
  const fs = new CachedFileSystem(rawFs);
  const lockFile = new LockFileUnderTest(fs);

  fs.writeFile(lockFile.lockFilePath, '188');
  const promise = lockFile.lock(async () => lockFile.log.push('fn()'));
  expect(lockFile.log).toEqual(['create()']);
  expect(lockFile.getLogger().logs.info).toEqual([...]);

  rawFs.writeFile(lockFile.lockFilePath, '444');
  await moveToNextIteration();
  await moveToNextIteration();
  expect(lockFile.getLogger().logs.info).toEqual([...]]);

  ...
});

[kara]

@petebacondarwin Are you still waiting on a review from @alxhub or is this ready to presubmit/merge?

[petebacondarwin]

Just a presubmit please @kara

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}