Skip to content
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 broken tests on windows #179

Merged
merged 2 commits into from
May 16, 2013
Merged

fix broken tests on windows #179

merged 2 commits into from
May 16, 2013

Conversation

kevinarcher
Copy link
Contributor

This PR may seem unimportant but the test suite should pass on windows.

@@ -14,7 +14,13 @@ class AbstractFilesystemResolverTest extends AbstractTest
public function testStoreCyrillicFilename()
{
$image = $this->fixturesDir.'/assets/АГГЗ.jpeg';
$data = file_get_contents($image);

$data = @file_get_contents($image);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this error suppressed?

@kevinarcher
Copy link
Contributor Author

With regards to the first 3 comments.. These relate to the fact the file_get_contents does not work on Windows with UTF-8 file names.. I attempted to find a way around it without any luck.

So, either the test could be skipped on windows .. because it is known that it's not supported. I didn't want to leave it as a failure because it could be misleading that it's a "failure" when it's actually expected behavior on Windows. Let me know which you prefer.

@havvg
Copy link
Contributor

havvg commented May 13, 2013

havvg added a commit that referenced this pull request May 16, 2013
@havvg havvg merged commit 9d59494 into liip:master May 16, 2013
@havvg
Copy link
Contributor

havvg commented May 16, 2013

Thank you!

@kevinarcher kevinarcher deleted the tests/windows-support branch May 18, 2013 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Attn: Critical This issue or PR is critical and should be rushed into a new release ASAP.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants