fuel / core

Fuel PHP Framework - The core of the Fuel v1 framework
http://fuelphp.com
813 stars 345 forks source link

Tests fail on Windows #1765

Closed kenjis closed 9 years ago

kenjis commented 10 years ago

FuelPHP 1.7.2 and 1.8/develop PHP 5.5.11

Tests Running...This may take a few moments.
PHPUnit 3.7.21 by Sebastian Bergmann.

Configuration read from C:\Users\Kenji\work\fuelphp\fuel\core\phpunit.xml

...............................................................  63 / 375 ( 16%)
............................................................... 126 / 375 ( 33%)
.......FFF............F........................................ 189 / 375 ( 50%)
............................................................... 252 / 375 ( 67%)
............................................................... 315 / 375 ( 84%)
............................................................

Time: 5 seconds, Memory: 14.25Mb

There were 4 failures:

1) Fuel\Core\Test_Debug::test_debug_dump_normally
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
 '  <script type="text/javascript">function fuel_debug_toggle(a){if(document.getElementById){if(document.getElementById(a).style.display=="none"){document.getElementById(a).style.display="block"}else{document.getElementById(a).style.display="none"}}else{if(document.layers){if(document.id.display=="none"){document.id.display="block"}else{document.id.display="none"}}else{if(document.all.id.style.display=="none"){document.all.id.style.display="block"}else{document.all.id.style.display="none"}}}};</script><div class="fuelphp-dump" style="font-size: 13px;background: #EEE !important; border:1px solid #666; color: #000 !important; padding:10px;"><h1 style="border-bottom: 1px solid #CCC; padding: 0 0 5px 0; margin: 0 0 5px 0; font: bold 120% sans-serif;">COREPATH/tests/debug.php @ line: 41</h1><pre style="overflow:auto;font-size:100%;"><strong>Variable #1:</strong>
 <i></i> <strong></strong> (Integer): 1

 <strong>Variable #2:</strong>
 <i></i> <strong></strong> (Integer): 2

 <strong>Variable #3:</strong>
 <i></i> <strong></strong> (Integer): 3

 </pre></div>'

C:\Users\Kenji\work\fuelphp\fuel\core\tests\debug.php:44

2) Fuel\Core\Test_Debug::test_debug_dump_by_call_user_func_array
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'<div class="fuelphp-dump" style="font-size: 13px;background: #EEE !important; border:1px solid #666; color: #000 !important; padding:10px;"><h1 style="border-bottom: 1px solid #CCC; padding: 0 0 5px 0; margin: 0 0 5px 0; font: bold 120% sans-serif;">COREPATH/tests/debug.php @ line: 53</h1><pre style="overflow:auto;font-size:100%;"><strong>Variable #1:</strong>
+'<div class="fuelphp-dump" style="font-size: 13px;background: #EEE !important; border:1px solid #666; color: #000 !important; padding:10px;"><h1 style="border-bottom: 1px solid #CCC; padding: 0 0 5px 0; margin: 0 0 5px 0; font: bold 120% sans-serif;">COREPATH/tests/debug.php @ line: 52</h1><pre style="overflow:auto;font-size:100%;"><strong>Variable #1:</strong>
 <i></i> <strong></strong> (Integer): 1

 <strong>Variable #2:</strong>
 <i></i> <strong></strong> (Integer): 2

 <strong>Variable #3:</strong>
 <i></i> <strong></strong> (Integer): 3

 </pre></div>'

C:\Users\Kenji\work\fuelphp\fuel\core\tests\debug.php:56

3) Fuel\Core\Test_Debug::test_debug_dump_by_call_fuel_func_array
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
 '<div class="fuelphp-dump" style="font-size: 13px;background: #EEE !important; border:1px solid #666; color: #000 !important; padding:10px;"><h1 style="border-bottom: 1px solid #CCC; padding: 0 0 5px 0; margin: 0 0 5px 0; font: bold 120% sans-serif;">COREPATH/base.php @ line: 491</h1><pre style="overflow:auto;font-size:100%;"><strong>Variable #1:</strong>
 <i></i> <strong></strong> (Integer): 1

 <strong>Variable #2:</strong>
 <i></i> <strong></strong> (Integer): 2

 <strong>Variable #3:</strong>
 <i></i> <strong></strong> (Integer): 3

 </pre></div>'

C:\Users\Kenji\work\fuelphp\fuel\core\tests\debug.php:80

4) Fuel\Core\Test_Fieldset::test_for_in_label
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'<form action="welcome/index" accept-charset="utf-8" method="post"><table><tr><td class=""></td><td class=""><input type="radio" value="0" id="form_gender_0" name="gender" /> <label for="form_gender_0">male</label><br /><input type="radio" value="1" id="form_gender_1" name="gender" checked="checked" /> <label for="form_gender_1">female</label><br /><span></span></td></tr></table></form>'
+'<form action="welcome/index" accept-charset="utf-8" method="post">
+<table><tr><td class=""></td><td class=""><input type="radio" value="0" id="form_gender_0" name="gender" /> <label for="form_gender_0">male</label><br /><input type="radio" value="1" id="form_gender_1" name="gender" checked="checked" /> <label for="form_gender_1">female</label><br /><span></span></td></tr>
+</table></form>
+'

C:\Users\Kenji\work\fuelphp\fuel\core\tests\fieldset.php:92

FAILURES!
Tests: 375, Assertions: 447, Failures: 4.
kenjis commented 10 years ago

Some tests fail on Windows, probably because of EOL code.

In my opinion, all EOL code in HTML output should be \n. But Fuel might not think so.

Which is better?

  1. HTML output EOL is \n
  2. HTML output EOL is PHP_EOL
WanWizard commented 9 years ago

I don't have a strong opinion on this subject (other than the fact that Microsoft should drop the CR, a newline is a newline ;-)).

Tests however should validate on all platforms. If the HTML generated is platform dependent, so should the expected values be.

Having said this, in the first test that fails, the code uses a HEREDOC, so the line-ends are part of the code. All Fuel source uses unix line-ends, so if your tests reveil that CRLF is returned, it suggests you did a checkout of the code without preserving line-ends.

This can't be dealt with, since it's outside the control of either the code or the test.

kenjis commented 9 years ago

If the HTML generated is platform dependent

Now it is, because Fuel uses PHP_EOL in code to generate HTML like below:

echo '<strong>Variable #'.$i.':</strong>'.PHP_EOL;

I guess you have an opinion, but you don't notice, do you? This is not because of Microsoft, only Fuel's code. (Of course if Microsoft used the same EOL as Unix, we don't have to think of this at all.)

For example, if the code above is right, we should fix the test code, because it uses "\n".

I could ask the same question in other way like this: Sould HTML generated be platform dependent or not?

WanWizard commented 9 years ago

You failed to see my point.

The code that gives the error in your opening post, is

$var = <<<HTML
line 1
line 2
line 4
HTML;

which means no PHP_EOL or hardcoded line-ends are used, but the line-ends you get are the one's used for the php file itself. Which for Fuel php files is always "\n", since we use unix line-ends as standard.

So the test should ALWAYS return a string with "\n" in it, no matter on which platform you run it. The only way you get CRLF is if you did a git clone without preserving line-ends.

So the code is right, and the test is right. Nothing to change.

As to what is generated, I have no real preference. I'd personally go for unix line-ends, since it's a byte less, and it's less typing. But I would say PHP_EOL is logical, so a Windows app generates CRLF (which Windows users would expect).