klein / klein.php

A fast & flexible router
MIT License
2.66k stars 290 forks source link

Update PHPUnit to 4.1.x #267

Closed SiebelsTim closed 9 years ago

SiebelsTim commented 9 years ago

PHPUnit added HHVM support in 4.x This also needed an update to php-code-coverage

This brings HHVM to 100% passrate in master

Rican7 commented 9 years ago

While this does allow for the tests to pass on HHVM, it causes a few tests to be skipped as they're marked as "Risky". I'll have to take a look at improving these tests.

This is a duplicate of #239, although I this PR is better as it doesn't change the PHP_CodeSniffer version for no reason. :P

SiebelsTim commented 9 years ago

Mh, these are probably poorly designed tests. I will look into that.

Rican7 commented 9 years ago

Yea, some of these tests are more or less "hacks" in order to get 100% code coverage.

SiebelsTim commented 9 years ago

Use 4.1.x for now, which has HHVM support, and doesn't has the feature that identify the risky tests.

Let's deal with 4.3.x later. When (if) this gets merged I can land the PR on HHVM to update klein :)

Rican7 commented 9 years ago

Hmm, still some funky output, but I know that its just because of an isolate hack anyway and this is worth getting in. So I'm merging.

Thanks again for these contributions!

SiebelsTim commented 9 years ago

You're great :) and fast :P

I get fastcgi_finish_request in the middle of nowhere. But it doesn't screw with any tests :)

Rican7 commented 9 years ago

Haha, thanks @SiebelsTim. You're the one making contributions! And they're small and easy-to-digest, so why not?

I've made an issue about the hacky tests so that I don't forget about them (and the context).

Thanks again! :+1:

SiebelsTim commented 9 years ago

@Rican7 These tests are risky, because https://github.com/chriso/klein.php/blob/master/src/Klein/Klein.php#L434 doesn't get closed when https://github.com/chriso/klein.php/blob/e68c9ef4a5e63003a560f370d502b19061f4a5c5/src/Klein/Klein.php#L635 throws the UnhandledException Exception.

I tried adding ob_end_clean and ob_end_flush to a finally block. That introduced test failures though. The dispatch function is huge (281 lines!!). That's why I didn't dig much more. Especially because I don't really know what that function is responsible for. But you might know when to close the buffer :)


Instructions on how to get a trace:

Add a simple new test to KleinTest.php, that doesn't check for the exception.

public function testBuffer() {
  $this->klein_app->response(function ($a, $b, $c) {
    throw new Exception("");
  });
  $this->klein_app->dispatch(); // This will throw, 
  // PHPUnit will show from where the Exception is raised.
  // It'll show you the line I linked ;-)
}
Rican7 commented 9 years ago

@SiebelsTim Ahh, good call. Thank you.

Yea, finally wasn't introduced until PHP 5.5, so I'll have to do something else.

And yes, that dispatch() method is THE WORST. I've been working on a huge re-factor of the dispatch and match process that pretty much converts that method into calling pieced-apart delegates. ;)

Thank you so much for discovering what's causing the issue. I appreciate it!

Rican7 commented 8 years ago

This was finally properly fixed by #340 and released in v2.1.1.

Thanks for all of your help, @SiebelsTim!