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

Issue/10991 Make unit tests pass on windows #12218

Merged
merged 11 commits into from
Nov 17, 2014
Merged

Conversation

nickvergessen
Copy link
Contributor

Further fixing to get #10991 done

Common Problems

uniqid() is not unique on Windows:

If you need a random string in your test please extend the \Test\TestCase class instead of \PHPUnit_Framework_TestCase (Once this PR is merged) and use $this->getUniqueID() instead.

Unclosed handles cause unlink() to fail:

On Windows php can not delete files which still have an open handle. Be sure to fclose() any handles you open, especially before deleting the file or directory.

chmod() does not work on Windows:

Windows does not have read/write permissions like UNIX. If you set chmod to 000 you can still write to the file/directory on Windows. In case you need to test something like that, Please mark the test as skipped for Windows, by adding the following code, right before the first assertion relying on the chmod change. We can still make all other assertions that happen till the change, so add it as late as possible:

    if (\OC_Util::runningOnWindows()) {
        $this->markTestSkipped('[Windows] chmod() does not work as intended on Windows.');
    }

@nickvergessen
Copy link
Contributor Author

@th3fallen @DeepDiver1975

@MorrisJobke @PVince81 should also fix the encryption tests on unix

@nickvergessen
Copy link
Contributor Author

@karlitschek while this PR looks quite huge, only 3 files apart from tests have been modified:

Would be nice if we can backport this to stable7 so tests can be run there much better (removes about 100 failures)

@karlitschek
Copy link
Contributor

O.K. please backport. Usually we don´t bckport tests but only important bugfixes. But let´s do this this time if you want.

@PVince81
Copy link
Contributor

Backporting tests is usually a good idea if it doesn't affect the functionality itself. This can help adding another layer of protection against regressions when backporting other stuff.

@PVince81
Copy link
Contributor

Code looks good, unit tests run fine on Linux 👍

@ghost
Copy link

ghost commented Nov 17, 2014

🚀 Test PASSed. 🚀
Refer to this link for build results (access rights to CI server needed):
https://ci.owncloud.org//job/pull-request-analyser-ng-simple/2689/
🚀 Test PASSed. 🚀

@MorrisJobke
Copy link
Contributor

Tests run fine. I now will have a look at the code and then will merge this :)

@nickvergessen
Copy link
Contributor Author

I will make a new PR for the backport, so this gets tested on jenkins correctly against stable7

@MorrisJobke
Copy link
Contributor

Code looks good 👍

@nickvergessen nickvergessen restored the issue/10991-fixes branch December 2, 2014 10:23
@nickvergessen nickvergessen deleted the issue/10991-fixes branch December 2, 2014 10:28
@lock lock bot locked as resolved and limited conversation to collaborators Aug 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants