apache / cordova-ios

Apache Cordova iOS
https://cordova.apache.org/
Apache License 2.0
2.15k stars 987 forks source link

refactor: replace superspawn with execa #1190

Closed raphinesse closed 2 years ago

raphinesse commented 2 years ago

Motivation and Context

Progresses https://github.com/apache/cordova/issues/16 and https://github.com/apache/cordova/issues/7

Description

Use execa for any subprocess spawning.

A few notes

execa has no equivalent of superspawn's printCommand option (if set to true, it emits a verbose log of the command being executed). In the current state of this PR, those verbose logs are removed without a substitute.

superspawn promises resolve to the captured stdout, while execa resolves to an object with more properties. In many of the cases where we spawn subprocesses, this does not matter, since we we only care about promise fulfillment.

superspawn allowed to subscribe to a running process' output by means of Q's progress method. This basically directly translates to subscribing to data events on the backing streams. So that is what we used to replace these calls. However, the existing code seems to assume that the data emitted are whole lines of output only. This is by no means guaranteed, so we added FIXME comments to the code.

Testing

codecov-commenter commented 2 years ago

Codecov Report

Merging #1190 (b01913e) into master (91d869a) will increase coverage by 0.24%. The diff coverage is 50.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1190      +/-   ##
==========================================
+ Coverage   74.90%   75.15%   +0.24%     
==========================================
  Files          13       13              
  Lines        1654     1658       +4     
==========================================
+ Hits         1239     1246       +7     
+ Misses        415      412       -3     
Impacted Files Coverage Δ
bin/templates/scripts/cordova/lib/run.js 23.07% <15.38%> (+0.34%) :arrow_up:
bin/templates/scripts/cordova/lib/Podfile.js 75.12% <22.22%> (+1.91%) :arrow_up:
bin/templates/scripts/cordova/lib/build.js 44.44% <50.00%> (+0.34%) :arrow_up:
bin/templates/scripts/cordova/lib/versions.js 89.36% <92.30%> (+0.98%) :arrow_up:
bin/templates/scripts/cordova/lib/listDevices.js 100.00% <100.00%> (ø)

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 91d869a...b01913e. Read the comment docs.

raphinesse commented 2 years ago

Thanks so much for the test!