Migrate PluginInstall plugin logics into standalone command

[issea1015]

Addresses: #9740

This PR will seclude plugin install out of the internals. Let me make similar changes for plugin uninstall once this PR is approved.

All other requirements and feedbacks written by @medikoo in the issue are applied in this PR but didn't much simplify the logic for these reasons:

• All procedures currently PluginInstall class performs looks necessary
  • https://github.com/serverless/serverless/pull/9942/files#diff-0eaec7938b8410881ba876915c3000cc4d3e167ab0a147ce4becac6e39cff580R50-R69
• The (class) structure which the logic is written into looks not much awkward

Please feel free to give me a further guidance :)

[medikoo]

@issea1015 Thank you, in general it looks very good!

I have few organization comments, also we need to configure some tests.

I think we need some unit test which doesn't depend on network: I imagine, that it may work as:

• Via fixture engine, setup some service fixture in which we will attempt to install plugin (some example:

  serverless/test/integration/plugin-install.test.js

  Line 16 in a8d1713

  await fixturesEngine.setup('function', {

  )
• Run our module in context of returned service path, with mocked (via proxyquire)child-process-ext/spawn (so no real network communication occurs)
• Confirm that:
  1. npm command was called with install command
  2. Plugin was added to serverless.yml

We need also integration test, but that one actually is already there:

serverless/test/integration/plugin-install.test.js

Line 16 in a8d1713

await fixturesEngine.setup('function', {

and it feels perfect good, so I wouldn't add anything here

[codecov]

Codecov Report

Merging #9942 (29b4a7e) into master (9e308bd) will decrease coverage by 0.58%.
The diff coverage is 68.96%.

❗ Current head 29b4a7e differs from pull request most recent head 846859a. Consider uploading reports for the commit 846859a to get more accurate results

@@            Coverage Diff             @@
##           master    #9942      +/-   ##
==========================================
- Coverage   86.18%   85.59%   -0.59%     
==========================================
  Files         329      332       +3     
  Lines       13065    13245     +180     
==========================================
+ Hits        11260    11337      +77     
- Misses       1805     1908     +103     

Impacted Files	Coverage Δ
lib/plugins/index.js	100.00% <ø> (ø)
lib/plugins/plugin/install.js	26.13% <ø> (-68.19%)	⬇️
commands/plugin-install.js	64.61% <64.61%> (ø)
lib/commands/plugin-management.js	69.23% <69.23%> (ø)
scripts/serverless.js	56.07% <100.00%> (+1.05%)	⬆️
lib/plugins/aws/deployList.js	71.42% <0.00%> (-20.24%)	⬇️
lib/cli/render-help/general.js	98.11% <0.00%> (-1.89%)	⬇️
lib/Serverless.js	68.88% <0.00%> (-0.17%)	⬇️
... and 26 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9e308bd...846859a. Read the comment docs.

[issea1015]

@medikoo Thanks for the clear description of all your feedbacks! I think I covered all of them in recent commits. Hope you take a look again :)

[medikoo]

@issea1015 that looks impressive! It's way way clearer now. I still have some remarks, but in general we're close to have it right.

Let me know what you think of my suggestions

[issea1015]

@medikoo Thanks for feedbacks with full background reasoning! It was seamless to follow them up. Let me know how it is now :)

[medikoo]

@issea1015 that looks great! I have just few last notes, and we should be good to go

[issea1015]

@medikoo Thanks for the step-by-step guidance all the while. That was impressive!

[medikoo]

Thank you @issea1015 ! All good, still to completely keep support for internal plugin install handling, some other changes also need to be reverted. I've commented under them specifically

[issea1015]

@medikoo Gotcha 🤦 I might have rushed in fact 😛
Done!

[medikoo]

That looks great @issea1015 Thank you!

I assume you'd follow with plugin uninstall with next PR?

[issea1015]

I assume you'd follow with plugin uninstall with next PR?

@medikoo leave it to me!