Symfony\Component\ClassLoader\ClassMapGenerator dump test #5330
Conversation
|
|
||
| public function setUp() | ||
| { | ||
| $this->workspace = rtrim(sys_get_temp_dir(), DIRECTORY_SEPARATOR).DIRECTORY_SEPARATOR.time().rand(0, 1000); |
There was a problem hiding this comment.
It would be better to not do that in the setUp method as most of the tests do not use this. What about just creating the directory when you actually need it?
|
Fixed, thanks for the review! |
| */ | ||
| public function testDump($directory, $expected) | ||
| { | ||
| $file = $this->workspace.DIRECTORY_SEPARATOR.'file'; |
There was a problem hiding this comment.
don't see why you are using DIRECTORY_SEPARATOR here, windows has no problem to deal with / in file paths
There was a problem hiding this comment.
I did it because it's how FilesystemTest is written. I don't know much about PHP issues on Windows and I don't want to reinvent the wheel :)
There was a problem hiding this comment.
the only places were we have to use DIRECTORY_SEPARATOR is when we do a string comparison with a path (which is done in FilesystemTest), as string definitely have an issue to consider / and \ being equivalent :)
There was a problem hiding this comment.
or for this for example : rtrim(sys_get_temp_dir(), DIRECTORY_SEPARATOR)but if you start to use it everytime you have to deal with paths, it becomes crazy
…(PR #5330) Commits ------- f1970fa dump test Discussion ---------- Symfony\Component\ClassLoader\ClassMapGenerator dump test Hi, 100 percent test coverage for ClassMapGenerator :) Best regards, Michal --------------------------------------------------------------------------- by eventhorizonpl at 2012-08-24T07:47:24Z Fixed, thanks for the review!
Hi,
100 percent test coverage for ClassMapGenerator :)
Best regards,
Michal