New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix tests on windows #2053
fix tests on windows #2053
Changes from 4 commits
30085ef
7b0d379
e9d2548
e8da026
1d7c1f1
cd08f3e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -38,10 +38,15 @@ private function csstest($input, $expected_css, $expected_less) { | |
$this->assertEquals($expected_less, $less); | ||
} | ||
|
||
// make relative | ||
private function importPath($path) { | ||
return isWindows() ? preg_replace('#(^.*[\\\\])#','', $path) : preg_replace('#(^.*[/])#','', $path); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not a fan of the tertiary operator here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what do you mean? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I prefer to have a proper if-then-else construct here instead of the |
||
} | ||
|
||
public function test_basic() { | ||
$this->setUpFiles(); | ||
|
||
$import = preg_replace('#(^.*[/])#','',$this->import); | ||
$import = $this->importPath($this->import); | ||
$in_css = '@import "'.$import.'";'; | ||
$in_less = '@foo: "bar"; | ||
content: @foo;'; | ||
|
@@ -56,12 +61,12 @@ public function test_basic() { | |
public function test_subdirectory() { | ||
$this->setUpFiles('/foo/bar'); | ||
|
||
$import = preg_replace('#(^.*[/])#','',$this->import); | ||
$import = $this->importPath($this->import); | ||
$in_css = '@import "'.$import.'";'; | ||
$in_less = '@foo: "bar"; | ||
content: @foo;'; | ||
|
||
$expected_css = '@import "/foo/bar/'.$import.'";'; | ||
$expected_css = isWindows() ? '@import "\\foo\\bar/'.$import.'";' : '@import "/foo/bar/'.$import.'";'; | ||
$expected_less = 'content: "bar";'; | ||
|
||
io_saveFile($this->import, $in_less); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,20 @@ | |
* Initialize some defaults needed for DokuWiki | ||
*/ | ||
|
||
if (!function_exists('isWindows')) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why the function_exists check here? |
||
// checks if it is windows OS | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. proper doc blocks should be used. |
||
function isWindows() { | ||
return (strtoupper(substr(PHP_OS, 0, 3)) === 'WIN') ? true : false; | ||
} | ||
} | ||
|
||
if (!function_exists('w2u')) { | ||
// convert windows path to unix-like on windows OS | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. proper doc block should be used |
||
function w2u($filename) { | ||
return isWindows() ? str_replace('\\', '/', $filename) : $filename; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure this is needed/useful outside of tests. PHP does not care for forward or backward slashes when accessing files. So this is only ever needed when comparing files read from the file system with predefines (as done in the tests). So if this only needed for testing it could be moved to the test's utility functions. |
||
} | ||
|
||
/** | ||
* timing Dokuwiki execution | ||
* | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think for tests which's task is testing something that does not work/exist in Windows, it would make more sense to just mark the test as skipped. Eg. here the idea is to test the encoding in the name, but since the name is omitted on windows, there is nothing to test that hasn't been tested above already.