CB-14140 Switch to using fs-extra in favour of shelljs

[dpogue]

Nowhere near ready to review or merge, but opening so I can see how the tests are faring on Travis/Appveyor...

Platforms affected

What does this PR do?

What testing has been done on this change?

Checklist

• Reported an issue in the JIRA database
• Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.
• Added automated test coverage as appropriate for this change.

[brodybits]

Linking this wonderful change in progress to Apache CB-14140 (https://issues.apache.org/jira/browse/CB-14140).

[raphinesse]

I just checked, and other than in integration-tests/HooksRunner.spec.js this does not conflict with #619 😌 And the conflicts that occurred seemed to be easily resolvable.

Should I merge #619 so that you can rebase this?

[brodybits]

Should I merge #619 so that you can rebase this?

+1 I just gave a review with approval on #619.

Would #619 help resolve the CI test failures?

[raphinesse]

@dpogue I've pushed a rebased version of this to my fork

[dpogue]

@raphinesse Thanks, I won't be able to look at this (or much of anything else) until sometime next week

[brodybits]

@dpogue I've pushed a rebased version of this to my fork

@raphinesse any reason you don't want to finish this one?

Nice work on #619 BTW, wonder if we should apply similar updates to some of the other Cordova repos?

[raphinesse]

@raphinesse any reason you don't want to finish this one?

I'm still working on other cordova stuff (mainly reducing Q-usage) and actually have to get a presentation ready until Tuesday. So time is sparse for me too right now. I'm happy to review this when it's finished.

Nice work on #619 BTW, wonder if we should apply similar updates to some of the other Cordova repos?

See my reply in #619.

[raphinesse]

@dpogue Is 3cb7f1e ready to merge? If so, we could land it in a separate PR for now.

[raphinesse]

I rebased the changes onto current master. I think we should work on getting this PR merged. It becomes too easily outdated, given the huge impact.

[dpogue]

yeah... I wish I knew why the dependency order was changing in those plugin install tests with these changes :(

[raphinesse]

Unit tests pass now. The problem was that the test relied on the shelljs stubbing for controlling behaviour of cordova-common too.

During debugging, I stumbled across stuff that had been removed in #649 and #666, so we should get them merged ASAP too 😬

[raphinesse]

@dpogue Thanks for the reviews! I'm going to rebase this and see if I can fix (some of) the failing E2E tests.

[raphinesse]

OK, one more failing test in HooksRunner.spec on Node 6 only, but other than that tests are passing. 🎆

[raphinesse]

It is done! 🍾

[dpogue]

🎉 Thanks so much for your work on finishing this! 🙇‍♂️

Do you want these squashed when merging, or kept as distinct commits for historical clarity?

[raphinesse]

I kept them separate for the review. Before merging, I'd manually squash most commits. So just let me know when this is good to go and I'll clean it up and get it merged.

[raphinesse]

I squashed some of the commits and will merge (with merge commit) as soon as the CI tests are done.

[oliversalzburg]

Just to link these issues: https://github.com/apache/cordova-common/issues/64

As cordova-lib also uses the same fs-extra copy functions, this change is just as much of an issue.