Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Path in phpunit.xml.dist is platform dependent #585

Closed
jmini opened this issue Jul 17, 2018 · 4 comments · Fixed by #610
Closed

Path in phpunit.xml.dist is platform dependent #585

jmini opened this issue Jul 17, 2018 · 4 comments · Fixed by #610

Comments

@jmini
Copy link
Member

jmini commented Jul 17, 2018

In PR #578 the samples were generated on windows, and when the ensure-up-to-date scripts run (see #80) we have the following diff on Shippable:

diff --git a/samples/server/petstore/php-slim/phpunit.xml.dist b/samples/server/petstore/php-slim/phpunit.xml.dist
index a9fd1a3ad..4a44f5ac1 100644
--- a/samples/server/petstore/php-slim/phpunit.xml.dist
+++ b/samples/server/petstore/php-slim/phpunit.xml.dist
@@ -11,16 +11,16 @@
 >
     <testsuites>
         <testsuite name="Apis">
-            <directory>.\test\Api</directory>
+            <directory>./test/Api</directory>
         </testsuite>
         <testsuite name="Models">
-            <directory>.\test\Model</directory>
+            <directory>./test/Model</directory>
         </testsuite>
     </testsuites>
     <filter>
         <whitelist processUncoveredFilesFromWhitelist="true">
-            <directory suffix=".php">.\lib\Api</directory>
-            <directory suffix=".php">.\lib\Model</directory>
+            <directory suffix=".php">./lib/Api</directory>
+            <directory suffix=".php">./lib/Model</directory>
         </whitelist>
     </filter>
 </phpunit>
\ No newline at end of file

Our output should be platform independent.

@wing328
Copy link
Member

wing328 commented Jul 17, 2018

One "workaround" is to put phpunit.xml.dist in .gitignore as the file is not being used in the test at the moment and I seldom see changes made to that particular file

cc @ackintosh

@ackintosh
Copy link
Contributor

Oh...

Should we use / instead of File.separator?

phpunit.xml.mustache#L23
modelSrcPath

@jmini
Copy link
Member Author

jmini commented Jul 19, 2018

I do not think that .gitignore can be a solution. If I remember well how git works you have 2 situations:

  1. the file is in the repo, then .gitignore is ignored, meaning that you will see when there are some changes and I believe that the file is also displayed when you do a git status.
  2. the file is not in the repo, to add it you need to do a git add --force and then you are back to case 1.

I tend to prefer @ackintosh solution, because I consider that generator output should be independent from the platform. Imagine cases where you have Win/Mac/Linux in the same team (it is my case). I do not want to commit changes like changing path/to/file.php to path\to\file.php just because of the OS of the user doing the update.

@ackintosh
Copy link
Contributor

@jmini Thanks for the details! I'll file a PR.

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants