Tests are failing on Windows / Fix line ending handling

[jrfnl]

All test builds against the Windows OS are currently failing and have been for a long time.

If we look back at the last time the workflow was run completely, this can be seen in the annotations below the summary, which show that the Windows test runs all failed.
https://github.com/phpDocumentor/ReflectionDocBlock/actions/runs/370378454

This was/is "hidden" due to the continue-on-error: true in the "Run PHPUnit" step in the unit test job, which will still mark the job as "passed", even though in reality it failed.

---

Details of the test failures (click to fold out)

There was 1 error:

1) phpDocumentor\Reflection\DocBlockFactoryTest::testTagsAreInterpretedUsingFactory
Mockery\Exception\NoMatchingExpectationException: No matching handler found for Mockery_1_phpDocumentor_Reflection_DocBlock_TagFactory::create('@author Mike van Riel <me@mikevanriel.com> This is with
  multiline description.', object(phpDocumentor\Reflection\Types\Context)). Either the method was unexpected or its arguments matched no expected argument list for this method

Objects: ( array (
  'phpDocumentor\\Reflection\\Types\\Context' => 
  array (
    'class' => 'phpDocumentor\\Reflection\\Types\\Context',
    'properties' => 
    array (
    ),
  ),
))

D:\a\ReflectionDocBlock\ReflectionDocBlock\src\DocBlockFactory.php:244
D:\a\ReflectionDocBlock\ReflectionDocBlock\src\DocBlockFactory.php:104
D:\a\ReflectionDocBlock\ReflectionDocBlock\tests\unit\DocBlockFactoryTest.php:203

--

There were 11 failures:

1) phpDocumentor\Reflection\DocBlockFactoryTest::testDescriptionsRetainFormatting
Failed asserting that two objects are equal.
--- Expected
+++ Actual
@@ @@
 phpDocumentor\Reflection\DocBlock\Description Object (
-    'bodyTemplate' => 'This is a multiline Description\r\n
-    that contains a code block.\r\n
-    \r\n
+    'bodyTemplate' => 'This is a multiline Description\n
+    that contains a code block.\n
+    \n
         See here: a CodeBlock'
     'tags' => Array ()
 )

D:\a\ReflectionDocBlock\ReflectionDocBlock\tests\unit\DocBlockFactoryTest.php:171

2) phpDocumentor\Reflection\DocBlock\DescriptionFactoryTest::testIfSuperfluousStartingSpacesAreRemoved
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
 #Warning: Strings contain different line endings!
 'This is a multiline
 description that you commonly
 see with tags.
-
+
     It does have a multiline code sample
     that should align, no matter what
-
+
 All spaces superfluous spaces on the
 second and later lines should be
 removed but the code sample should
 still be indented.'

D:\a\ReflectionDocBlock\ReflectionDocBlock\tests\unit\DocBlock\DescriptionFactoryTest.php:195

3) phpDocumentor\Reflection\DocBlock\SerializerTest::testReconstructsADocCommentFromADocBlock
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
 #Warning: Strings contain different line endings!
-'/**
- * This is a summary
- *
- * This is a description
- *
- * @unknown-tag Test description for the unknown tag
+'/**
+ * This is a summary
+ *
+ * This is a description
+ *
+ * @unknown-tag Test description for the unknown tag
  */'

D:\a\ReflectionDocBlock\ReflectionDocBlock\tests\unit\DocBlock\SerializerTest.php:66

4) phpDocumentor\Reflection\DocBlock\SerializerTest::testAddPrefixToDocBlock
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
 #Warning: Strings contain different line endings!
-'aa/**
-aa * This is a summary
-aa *
-aa * This is a description
-aa *
-aa * @unknown-tag Test description for the unknown tag
+'aa/**
+aa * This is a summary
+aa *
+aa * This is a description
+aa *
+aa * @unknown-tag Test description for the unknown tag
 aa */'

D:\a\ReflectionDocBlock\ReflectionDocBlock\tests\unit\DocBlock\SerializerTest.php:101

5) phpDocumentor\Reflection\DocBlock\SerializerTest::testAddPrefixToDocBlockExceptFirstLine
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
 #Warning: Strings contain different line endings!
-'/**
-aa * This is a summary
-aa *
-aa * This is a description
-aa *
-aa * @unknown-tag Test description for the unknown tag
+'/**
+aa * This is a summary
+aa *
+aa * This is a description
+aa *
+aa * @unknown-tag Test description for the unknown tag
 aa */'

D:\a\ReflectionDocBlock\ReflectionDocBlock\tests\unit\DocBlock\SerializerTest.php:136

6) phpDocumentor\Reflection\DocBlock\SerializerTest::testWordwrapsAroundTheGivenAmountOfCharacters
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
 #Warning: Strings contain different line endings!
-'/**
- * This is a
- * summary
- *
- * This is a
- * description
- *
- * @unknown-tag
- * Test
- * description
- * for the
- * unknown tag
+'/**
+ * This is a
+ * summary
+ *
+ * This is a
+ * description
+ *
+ * @unknown-tag
+ * Test
+ * description
+ * for the
+ * unknown tag
  */'

D:\a\ReflectionDocBlock\ReflectionDocBlock\tests\unit\DocBlock\SerializerTest.php:177

7) phpDocumentor\Reflection\DocBlock\SerializerTest::testNoExtraSpacesAfterTagRemoval
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
 #Warning: Strings contain different line endings!
-'/**
- * @unknown-tag
+'/**
+ * @unknown-tag
  */'

D:\a\ReflectionDocBlock\ReflectionDocBlock\tests\unit\DocBlock\SerializerTest.php:201

8) phpDocumentor\Reflection\InterpretingDocBlocksTest::testInterpretingASimpleDocBlock
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
 #Warning: Strings contain different line endings!
-'This is a Description. A Summary and Description are separated by either
-two subsequent newlines (thus a whiteline in between as can be seen in this
-example), or when the Summary ends with a dot (`.`) and some form of
+'This is a Description. A Summary and Description are separated by either
+two subsequent newlines (thus a whiteline in between as can be seen in this
+example), or when the Summary ends with a dot (`.`) and some form of
 whitespace.'

D:\a\ReflectionDocBlock\ReflectionDocBlock\tests\integration\InterpretingDocBlocksTest.php:82

9) phpDocumentor\Reflection\InterpretingDocBlocksTest::testDescriptionsCanEscapeAtSignsAndClosingBraces
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
 #Warning: Strings contain different line endings!
-'You can escape the @-sign by surrounding it with braces, for example: @. And escape a closing brace within an
-inline tag by adding an opening brace in front of it like this: }.
-
-Here are example texts where you can see how they could be used in a real life situation:
-
-    This is a text with an {@internal inline tag where a closing brace (}) is shown}.
-    Or an {@internal inline tag with a literal {@link} in it}.
-
+'You can escape the @-sign by surrounding it with braces, for example: @. And escape a closing brace within an
+inline tag by adding an opening brace in front of it like this: }.
+
+Here are example texts where you can see how they could be used in a real life situation:
+
+    This is a text with an {@internal inline tag where a closing brace (}) is shown}.
+    Or an {@internal inline tag with a literal {@link} in it}.
+
 Do note that an {@internal inline tag that has an opening brace ({) does not break out}.'

D:\a\ReflectionDocBlock\ReflectionDocBlock\tests\integration\InterpretingDocBlocksTest.php:139

10) phpDocumentor\Reflection\ReconstitutingADocBlockTest::testReconstituteADocBlock
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
 #Warning: Strings contain different line endings!
-'/**
- * This is an example of a summary.
- *
- * And here is an example of the description
- * of a DocBlock that can span multiple lines.
- *
- * @see \phpDocumentor\Reflection\DocBlock\StandardTagFactory
+'/**
+ * This is an example of a summary.
+ *
+ * And here is an example of the description
+ * of a DocBlock that can span multiple lines.
+ *
+ * @see \phpDocumentor\Reflection\DocBlock\StandardTagFactory
  */'

D:\a\ReflectionDocBlock\ReflectionDocBlock\tests\integration\ReconstitutingADocBlockTest.php:41

11) phpDocumentor\Reflection\UsingTagsTest::testAddingYourOwnTagUsingAStaticMethodAsFactory
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
 #Warning: Strings contain different line endings!
-'/**
- * This is an example of a summary.
- *
- * @my-tag I have a description
+'/**
+ * This is an example of a summary.
+ *
+ * @my-tag I have a description
  */'

D:\a\ReflectionDocBlock\ReflectionDocBlock\tests\integration\UsingTagsTest.php:48

---

Most of the test failures in the Windows builds are due to the git check out of the code being done with core.autocrlf true and the line endings in the "expected" value not matching the line endings in the "actual" value.

There are two ways to look at these failures:

1. These are just test failures, line endings are of no consequence and should not be the concern of this repository.
   In that case, it could even be considered to remove the builds against Mac and Windows OSes completely.
2. This is a failure in the actual functionality of the repo.
   In that case, the use of new lines in various places in the code base will probably need to be changed to using PHP_EOL instead or, alternatively, there should be a check on the new line type of the input received and this same new line type should be used everywhere where new lines are added.

Opinions ?

[jaapio]

The newlines should be PHP_EOL which should be equal to what your OS is doing, we want to keep newlines. On output, basically it should be just like the input, but that can be tricky to store.

So I think we should fix those tests to use PHP_EOL and also in code when needed.

[jrfnl]

@jaapio I think it's actually more complicated than that and that this should probably be fixed by a maintainer of the repo, not me.

On Windows, when a repo is checked out, it can be checked out as:

1. core.autocrlf="true" = Check out Windows-style, commit Unix-style. This is the default on Windows.
2. core.autocrlf="input" = Check out as-is, commit Unix-style. This is what sane developers use 😂
3. core.autocrlf="false" = Check out as-is, commit as-is. Rarely used AFAIK.

If the relevant new line references in the code base are changed to PHP_EOL, things would work correctly for option [1], but would still be incorrect for [2].

So, I believe what is really needed is for what I described here:

there should be a check on the new line type of the input received and this same new line type should be used everywhere where new lines are added.

Basically, I think it should work like this:

1. When a docblock is received and pulled apart, store the original line ending in some way.
2. Normalize the line endings to "\n" for internal use.
3. And then if/when a docblock is put back together again, like in the Serializer, the original line ending should be used again, replacing the normalized line endings.

But to implement this, would require a refactor in various places and I'm loath to do this as this is not a repo I use and I can't fully gauge the impact of the various ways these changes can be made.

In a lot of ways I'm still inclined to say that line endings should not be the concern of this repo and should be fixed up via a coding standards run over (re-)generated code. In that case, it would be a test-only change which I'd be happy to make.

[jaapio]

I agree with you that line-endings should not be in scope of this library. I think we need to stick to the behavior of having \n as line-ending.

The vast majority of our users are using this library to parse docblocks and not to generate code. If they are using it to generate code some other things will be broken as well. Like allignment etc. So they will have to run a style fixer anyway.

I'm going to fix this by making the tests static.

[jaapio]

While working on a fix I Discovered this library does something odd, it will keep the line-endings in some situations which makes sense for speed. But not for output.

[jrfnl]

@jaapio Yup, I'd noticed that as well when I was looking at this before.

[jaapio]

I ended up with the following fixes:

• Split descriptions by any newlines \n or \r\n
• Internally we will keep using \n to keep things simple.
• The serializer used to print docblocks is now configurable. By default it will use \n

Since line endings could be mixed in a file, it would be hard to detect what we should output. If people want to change the output, use a proper tool to do that. :-)