New command: app:new:from:drupal7

[wimleers]

Motivation
ACLI already has a app:new:local command that spins up a brand new Drupal project.

Wouldn't it be great if it also helped you spin up a new Drupal 9+ project that helps migrating from Drupal 7?

Proposed changes
Introduce acli app:new:from:drupal7.

All code has been battle-hardened over years of use in production on Acquia Cloud. Please check the commits in sequence, because I designed them to tell a gradual story:

1. code to inspect a Drupal 7 site's installed modules
2. introduce the new ACLI command
3. add unit tests
4. add integration tests

Until the full set of recommendations is released, this will use only a minimal set of recommendations, primarily consisting of some Drupal 7 → 9 migration recommendations for Acquia modules.

This is a necessary step in the way of open sourcing https://www.acquia.com/products/drupal-cloud/acquia-migrate-accelerate.

Alternatives considered
N/A

Testing steps
Point it at a Drupal 7 site and observe that it generates a valid composer.json that results in an installable Drupal 9+ site. See the integration tests for various sample Drupal 7 sites at tests/phpunit/src/Commands/App/NewFromDrupal7CommandTest.php, with fixtures at tests/fixtures/drupal7.

[codecov]

Codecov Report

Patch coverage: 91.61% and project coverage change: -0.02% ⚠️

Comparison is base (fe86843) 91.77% compared to head (c214e45) 91.76%.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #1543      +/-   ##
============================================
- Coverage     91.77%   91.76%   -0.02%     
- Complexity     1663     1809     +146     
============================================
  Files           110      124      +14     
  Lines          5957     6470     +513     
============================================
+ Hits           5467     5937     +470     
- Misses          490      533      +43     

Files Changed	Coverage Δ
...pp/From/Recommendation/UniversalRecommendation.php	75.00% <75.00%> (ø)
src/Command/App/NewFromDrupal7Command.php	77.24% <77.24%> (ø)
...mmand/App/From/Safety/StructuredArrayValidator.php	84.37% <84.37%> (ø)
...c/Command/App/From/Safety/ArrayValidationTrait.php	90.90% <90.90%> (ø)
.../From/Recommendation/AbandonmentRecommendation.php	96.42% <96.42%> (ø)
src/Command/App/From/Composer/ProjectBuilder.php	100.00% <100.00%> (ø)
src/Command/App/From/Configuration.php	100.00% <100.00%> (ø)
src/Command/App/From/JsonResourceParserTrait.php	100.00% <100.00%> (ø)
.../App/From/Recommendation/DefinedRecommendation.php	100.00% <100.00%> (ø)
...ommand/App/From/Recommendation/Recommendations.php	100.00% <100.00%> (ø)
... and 4 more

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[anavarre]

Hey Wim! Thanks for your contribution. @danepowell is out for ~2 weeks so expect radio silence for a little while. Making visible to @mikemadison13 for AMA efforts.

[wimleers]

Hey Wim! Thanks for your contribution. @danepowell is out for ~2 weeks so expect radio silence for a little while. Making visible to @mikemadison13 for AMA efforts.

Yep, I'm aware 😊

[wimleers]

The job was canceled because "ubuntu-22_04_8_1_pcov" failed.

🤷 Looks like GitHub Actions is broken today…

[wimleers]

At least in 987ce00, there were some genuine failures:

1)                                                                     
         Acquia\Cli\Tests\Commands\App\From\AbandonmentRecommendationTest::testVersionCon
         straint                                                                
         "@covers getVersionConstraint" is invalid      

Fixed in 9a0814b.

Rooting for full green again after the weird auto-canceling CI jobs! 🤞

[wimleers]

Green again!

Just "failing" on reduced code coverage: from 91.56% for the entire project to 90.93%.

From 81.31% in 5ec7893, to 82.55% in 9a0814b, to 84.17% test coverage in e22a60b for new code. I have one more idea to get it up significantly, just pushed 7d88c51 for that 🤞

[wimleers]

🥳 91.58% of target 91.56% 😄

[wimleers]

Aha, looks like there was a failure on Windows too. getLocation() only works on UNIX-style paths today:

Script @unit-parallel was called via unit
1) Acquia\Cli\Tests\Commands\App\NewFromDrupal7CommandTest::testNewFromDrupal7Command with data set "training.acquia.com" ('D:\a\cli\cli/tests/fixtures/d...s.json', 'D:\a\cli\cli/config/from_d7_r...s.json', 'D:\a\cli\cli/tests/fixtures/d...d.json')
Script @unit was called via test
TypeError: Acquia\Cli\Command\App\NewFromDrupal7Command::getLocation(): Return value must be of type string, bool returned

Attempted a fix.

[wimleers]

• before a694e0e: D:\a\cli\cli/tests/fixtures/
• after a694e0e: D:/a/cli/cli/tests/fixtures/

👆 Now paths were formatted in a way accepted by Windows. But our logic to detect absolute paths vs not was wrong. That's what da056f6 is aiming to fix.

[wimleers]

🥳 Green!

For fun, I also checked the mutation testing output, which I've never before seen. It's pretty cool, and it shows that the tests catch most things:

279 mutations were generated:
     216 mutants were killed
       0 mutants were configured to be ignored
       0 mutants were not covered by tests
      62 covered mutants were not detected
       0 errors were encountered
       0 syntax errors were encountered
       1 time outs were encountered
       0 mutants required more time than configured

Metrics:
         Mutation Score Indicator (MSI): 77%
         Mutation Code Coverage: 100%
         Covered Code MSI: 77%

Most of these are for the untestable "inspect a live Drupal 7 site" code path. But not all. One of them is pretty silly and is only due to a limitation in PHP <8:

Warning: Escaped Mutant for Mutator "DecrementInteger":

--- Original
+++ New
@@ @@
     {
         assert(is_resource($resource));
         $json = stream_get_contents($resource);
-        return json_decode($json, TRUE, 512, JSON_OBJECT_AS_ARRAY | JSON_THROW_ON_ERROR);
+        return json_decode($json, TRUE, 511, JSON_OBJECT_AS_ARRAY | JSON_THROW_ON_ERROR);
     }
 }


Warning: Escaped Mutant for Mutator "IncrementInteger":

--- Original
+++ New
@@ @@
     {
         assert(is_resource($resource));
         $json = stream_get_contents($resource);
-        return json_decode($json, TRUE, 512, JSON_OBJECT_AS_ARRAY | JSON_THROW_ON_ERROR);
+        return json_decode($json, TRUE, 513, JSON_OBJECT_AS_ARRAY | JSON_THROW_ON_ERROR);
     }
 }

Just switching to PHP >=8's named arguments fixes this!

[wimleers]

Entire codebase with this PR:

3646 mutations were generated:
    1364 mutants were killed
       0 mutants were configured to be ignored
       0 mutants were not covered by tests
     772 covered mutants were not detected
      69 errors were encountered
       0 syntax errors were encountered
      20 time outs were encountered
    1421 mutants required more time than configured

Metrics:
         Mutation Score Indicator (MSI): 65%
         Mutation Code Coverage: 100%
         Covered Code MSI: 65%

☝️ this PR means a net improvement to the mutation score per my previous comment: ~77% vs ~65%. 😊 (Would be cool to have a target to not regress against this too?)

[wimleers]

a9c042e fixes a bug that I found while updating an integration test of a private repo to rely on the new ACLI command being introduced here.

That integration test does point to a real, live Drupal 7 site. It crashed like so:

php ./bin/acli app:new:from:drupal7 --drupal7-directory $D7_DIR --recommendations "$RECOMMENDATIONS_DIR/recommendations.json" --directory /tmp/build
We strive to give you the best tools for development.
You can really help us improve by sharing anonymous performance and usage data.
 Would you like to share anonymous performance usage and data? (yes/no) [yes]:
 > Awesome! Thank you for helping!

 [ERROR] The given --drupal7-uri value does not correspond to an installed sites
         directory and a sites.php file could not be located.        

[danepowell]

@wimleers this looks good to me, but it's still in draft state. Is this ready for review?

[danepowell]

Adam mentioned that we need to wait for approval on this.

[wimleers]

There's absolutely no reason left that it's in draft state 😅 I should've marked this as ready for review ever since #1543 (comment) — sorry!

[wimleers]

The actual tests are still green after merging in upstream, but:

Failing after 1s — 91.61% of diff hit (target 91.77%)

Back in June, the target was 91.56% 😬 I hope that won't hold this up? 😇