Windows - Wrong result from DownMigration task (apparently related to class file CR/LF parsing)

[flap152]

• Laravel Version: 10.16
• PHP Version: 8.1.13
• Shift CLI Version: 0.2.8
• Platform: Windows 10

Issue:

I get errors using shift-cli with the DownMigration task. It removed the wrong lines and left the code in error. It chopped of the end of a comment block, part of the up method and left the down method intact) Investigating, I thought I had some control over the problem (the more comment lines I had, the more it removed, but still in the wrong place) Now to isolate the problem, I tried to run it on a small project, but thought it would be better to confirm the intended behaviour, so I cloned the package and ran the tests. I got errors related to path separator and line endings CR/LF. The line separator "bug" seemed to be in the tests themselves. But I was not sure about the CR / CRLF problem. Are shift-cli tests only expected to run on CI so not Windows?

So I found the parsing of the migration wrongly "created new lines" for EACH CR and LF, then removed the wrong lines because the indexing is wrong. This change fixed the original problem:

diff --git a/src/Tasks/DownMigration.php b/src/Tasks/DownMigration.php
--- a/src/Tasks/DownMigration.php   (revision 451ca14d15171080d6029caca41103791779d58a)
+++ b/src/Tasks/DownMigration.php   (revision b33c2de02b2a33b13b95945ec2069ee63b79afa3)
@@ -84,7 +84,7 @@

     private function removeDownMethod(string $contents): ?string
     {
-        $file = File::fromString($contents);
+        $file = File::fromString($contents, true);
         $class = $this->parseClass($file->contents());
         if (! isset($class['methods']['down'])) {
             return null;

Command:

on shift-cli clone:

php ./vendor/phpunit/phpunit/phpunit  --configuration  phpunit.xml.dist

on my project:

sh
shift-cli -vvv run  --path C:\dev\Gits\apps\unify\packages\residential-base-data\database\migrations\2018_10_16_130501_create_res_checkpoints_table.php -- down-migration

Output:

PHPUnit 10.3.5 by Sebastian Bergmann and contributors.                                                            

Runtime:       PHP 8.1.13                                                                                         
Configuration: C:\dev\Gits\gitlibs\laravel-shift-cli\phpunit.xml.dist                                             

.......F......F........                                           23 / 23 (100%)                                  

Time: 00:00.933, Memory: 18.00 MB                                                                                 

There were 2 failures:                                                                                            

1) Tests\Feature\Tasks\CheckLintTest::it_returns_failure_and_leaves_comment_when_syntax_errors_are_found          
Failed asserting that two arrays are identical.                                                                   
--- Expected                                                                                                      
+++ Actual                                                                                                        
@@ @@                                                                                                             
 Array &0 [                                                                                                       
-    0 => 'Line 1: unexpected double-quote mark, expecting "," or ";"',                                           
+    0 => 'Line : ',                                                                                              
 ]                                                                                                                

C:\dev\Gits\gitlibs\laravel-shift-cli\tests\Feature\Tasks\CheckLintTest.php:57                                    
C:\dev\Gits\gitlibs\laravel-shift-cli\vendor\phpunit\phpunit\phpunit:99                                           

2) Tests\Feature\Tasks\DownMigrationTest::it_removes_down_method                                                  
Failed asserting that two strings are equal.                                                                      
--- Expected                                                                                                      
+++ Actual                                                                                                        
@@ @@                                                                                                             
 class SimpleMigration extends Migration\r\n                                                                      
 {\r\n                                                                                                            
     /**\r\n                                                                                                      
-     * Run the migrations.\r\n                                                                                   
-     *\r\n                                                                                                       
-     * @return void\r\n                                                                                          
-     */\r\n                                                                                                      
+     * Run the migrations.\r                                                                                     
     public function up()\r\n                                                                                     
     {\r\n                                                                                                        
         Schema::table('videos', function (Blueprint $table) {\r\n                                                
             $table->integer('runtime')->nullable();\r\n                                                          
+        });\r\n                                                                                                  
+    }\r\n                                                                                                        
+\r\n                                                                                                             
+    public function down()\r\n                                                                                   
+    {\r\n                                                                                                        
+        Schema::table('videos', function (Blueprint $table) {\r\n                                                
+            $table->dropColumn('runtime');\r\n                                                                   
         });\r\n                                                                                                  
     }\r\n                                                                                                        
 }\r\n                                                                                                            
 '                                                                                                                

C:\dev\Gits\gitlibs\laravel-shift-cli\vendor\laravel-shift\cli-sdk\src\Testing\InteractsWithProject.php:108       
C:\dev\Gits\gitlibs\laravel-shift-cli\tests\Feature\Tasks\DownMigrationTest.php:78                                
C:\dev\Gits\gitlibs\laravel-shift-cli\vendor\phpunit\phpunit\phpunit:99                                           

FAILURES!                                                                                                         
Tests: 23, Assertions: 152, Failures: 2, Warnings: 2.                                                             

[flap152]

Because it seems environment related, I'm thinking it might be related to core.autocrlf=true in git config, at least for the failing tests part. Let's see if I can test this theory.

[jasonmccreary]

So are you saying this changed fixed the problem:

-        $file = File::fromString($contents);
+        $file = File::fromString($contents, true);

[flap152]

Yes it did. In the meantime, I managed to confirm that if the fixtures use LF instead of CRLF, the test passes without or with our the modification. The task also works in my application if migrations use LF instead of CRLFs. I played around with git configs (autocrlf, text, eol...) until I got LFs or CRLFs and that is the difference. Manually toggling line-endings alos fixes/breaks the task. (PHPStorm user - not a git expert)

[jasonmccreary]

I think the proper solution would be to update File to only split on newlines by default. I'll look in the morning.

[jasonmccreary]

This should be resolved in 0.2.9. Let me know it not.

[flap152]

It worked. Thanks.