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

Replace SimpleTest with PHPUnit #96

Merged
merged 102 commits into from May 1, 2012
Merged

Replace SimpleTest with PHPUnit #96

merged 102 commits into from May 1, 2012

Conversation

dom-mel
Copy link
Collaborator

@dom-mel dom-mel commented Apr 18, 2012

This pull request migrates the SimpleTest test suite to PHPUnit. The initial ideas came from http://www.dokuwiki.org/devel:ideas:testing_system .

Detailed information about the testing framework can be found at https://github.com/dom-mel/dokuwiki/blob/phpunit/_test/README

Most unittests are migrated to PHPUnit, except for:

  • inc/html_hilight (runkit)
  • inc/parser/xhtml_htmlphp (runkit)
  • inc/indexer_idx_indexlengths (fs dependencies)
  • inc/mail_send (integration test)
  • inc/parser/parser_formatting
  • inc/parser/xhtml_links

In addition to unit tests, it is now possible to easily create integration tests. It allows to fake whole HTTP requests to DokuWiki with runtime inspection of those requests. You can also hook DokuWiki events. An example can be found in https://github.com/dom-mel/dokuwiki/blob/phpunit/_test/tests/test/hooks.test.php .

dom-mel and others added 30 commits April 1, 2012 18:31
plugin unit tests are in /lib/plugins/<pluginname>/_testing
@dom-mel
Copy link
Collaborator Author

dom-mel commented Apr 21, 2012

I've test the suite on windows. It worked ;-)

But for some reasons you may have to enable HTTPS support for php.
I've added a note in the README.

@splitbrain
Copy link
Collaborator

Nice, could you add a simple description on how to install phpunit on Windows?

@michitux
Copy link
Collaborator

I think this is great, the only thing that didn't work for me (after adapting or removing the plugin tests I had) is the http client test, somehow it wasn't able to reach httpbin.org and I also couldn't test it alone, then it fails with an error that it can't find the HTTPClient class. But as we had similar and most probably even more problems with the old testing system I don't think this is a reason against merging this branch.

@dom-mel
Copy link
Collaborator Author

dom-mel commented Apr 26, 2012

Sounds like you habe not installed the PHP SSL handler, can you provide the stacktrace?

this fixes the HTTP tests which do test the base class directly instead
of the DokuHTTPClient subclass
@splitbrain
Copy link
Collaborator

@michitux I fixed the problem with running the tests on its own and also check for SSL support now before running SSL tests. Could you try again?

according to Dominik Eckelmann  one of the tests fails on certain servers. I can't
reproduce it. If you can, please open a bug report with as much info as
possible.
@sarnowski
Copy link
Contributor

@splitbrain dom-mel's server is one of those machines which hangs on this test and iirc my old computer @ cosmocode also ran forever.

http://dwbuilds.linesofcode.org/phpunit-57.txt runs for nearly two days now ;-)

edit: we thought that it may be some cpu flags (maybe AES-NI[1]?)

[1] http://en.wikipedia.org/wiki/AES_instruction_set

@sarnowski
Copy link
Contributor

I tracked it down to the pmd5 and hmd5 tests - both are extreme cpu intensive (and do not finish on this test machine within an acceptable time frame).

@sarnowski
Copy link
Contributor

Another hint I want to mention is that the @author annotation is an alias for @group[1]. This results in the following group list:

$ phpunit --list-groups
PHPUnit 3.6.10 by Sebastian Bergmann.

Available test group(s):
 - Andreas Gohr <andi@splitbrain.org>
 - Denis Scheither <amorphis@uni-bremen.de>
 - __nogroup__
 - integration
 - internet
 - slow

This is a "feature" of phpunit which may be a good use case but is senseless right now (only the romanize utf8 test contains two @author annotations). Just give it a thought if this is ok or not.

[1] http://www.phpunit.de/manual/3.6/en/appendixes.annotations.html

@splitbrain
Copy link
Collaborator

Hmm, the PMD5 hashing method is supposed to be CPU intensive (similar to bcrypt) bt of course should still be reasonable. There's a iteraration loop in it (https://github.com/splitbrain/dokuwiki/blob/master/inc/PassHash.class.php#L346) my only explanation would be that this loop is running forever on your machine for some reason. Could you try adding some debug statements to see if that's the case and where it goes wrong?

@dom-mel
Copy link
Collaborator Author

dom-mel commented Apr 29, 2012

I've noticed the PMD5 hang on my Server and laptop. (Debian/Ubuntu - PHP 5.3.3-7+squeeze8/PHP5.3.--). On my Windows desktop it runs within seconds.

On laptop i tracked it down to [1] caused by a very high value in $iter.
[1] https://github.com/splitbrain/dokuwiki/blob/master/inc/PassHash.class.php#L339

To the CPU-Flag, my Windows desktop has AES-IN, laptop hasn't, server i don't know ;-)

@splitbrain if you want to try it yourself, i can give you access to my server.

@splitbrain
Copy link
Collaborator

Hmm, could this be a 64bit issue? Would be the bitshift in line 333 be affected by 64 vs. 32 bit?

@dom-mel
Copy link
Collaborator Author

dom-mel commented Apr 29, 2012

nope, all my machines are x64

@michitux
Copy link
Collaborator

I have the same problem here, debugging shows:

$salt (from parameter): abcdefgh12345678912345678912345678
$iterc ($salt[0]): a
$iter(strpos($itoa64,$iterc)): 38
$iter(1 << $iter) 274877906944

So I think this could be a 64bit issue, as on 32 bit systems I would expect an integer overflow in the calculation of $iter. Is that overflow intended, i.e. should we "simulate" that overflow on 64bit systems?

[edit]
Even though the PHP documentation says that integers are automatically converted to floats when the integer range is exceeded, that doesn't happen for bitshifts, some examples from my 64bit system here:

php > echo 1 << 62;
4611686018427387904
php > echo 1 << 63;
-9223372036854775808
php > echo 1 << 64;
1
php > echo 1 << 65;
2

On a 32bit VM I have the same effects around 32.

@sarnowski
Copy link
Contributor

I got the exact same results here. Initial $iter is 32 and will be shifted to 274877906944. 274 billion md5 iterations in php will take a really long time.

@michitux
Copy link
Collaborator

When I replace $iter by ($iter%32) in the bitshift, all tests pass for me, but I still think this isn't right. The first byte of the salt should be the iteration count and there should imho be some verification that this is a sane number, i.e. not larger than something like 12 (default is 8). I think for our test cases the problem is that the salt that is used for pmd5 has the wrong format, i.e. is not prefixed by a sane iteration count as the one that is generated by pmd5.

Input iteration counts are squared in the function and passing something
above 30 is giving integer overflows on 32 bit systems (and causes insane
iteration counts on 64bit systems).
@splitbrain
Copy link
Collaborator

I checked the original code. They had a check to abort on iteration counts > 30. The bitshift will square the input iteration count and anything above 30 doesn't make much sense anyway. Hell, more than 15 is probably overkill already.

The pushed changes should fix this.

@splitbrain
Copy link
Collaborator

And because nobody objected the idea in general, I'll just go ahead and merge this now.

splitbrain added a commit that referenced this pull request May 1, 2012
Replace SimpleTest with PHPUnit
@splitbrain splitbrain merged commit 3c0d44f into dokuwiki:master May 1, 2012
splitbrain added a commit that referenced this pull request Apr 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants