amphp / http-server

An advanced async HTTP server library for PHP, perfect for real-time apps and APIs with high concurrency demands.
https://amphp.org/http-server
MIT License
1.29k stars 100 forks source link

How to write end-to-end tests? #302

Closed enumag closed 4 years ago

enumag commented 4 years ago

While refactoring my project to use amphp/http-server I need to refactor my end-to-end tests. Previously I implemented those tests simply by calling Symfony's Kernel::handle($request) and then asserting the Response.

With Amphp I thought I'd avoid the HttpServer itself and just call Amp\Http\Server\Router::handleRequest() instead. In theory this should work just fine.

The problem I didn't expect is that Amp\Http\Server\Request wants an instance of Amp\Http\Server\Driver\Client. I'm not exactly sure what this "Client" is and how to create one for my PHPUnit tests.

Can you give me some advice how to create a "mock" Client for tests? I'm not exactly sure about it's purpose and what the Client needs to be capable of in tests environment. I did find the DefaultClientFactory class of course but it has many dependencies. I can easily create $errorHandler, $logger, $options and $timeoutCache ofc and use the Router instance as RequestHandler. But how do I create $socket that will work for tests? Or maybe a RemoteClient is not even the implementation I should be using in tests? But then again there is no other implementation...

kelunik commented 4 years ago

You can just create a mock. If you need any of the methods of the client, you'll have to mock these methods.

https://github.com/amphp/http-server-router/blob/c6a1731f3833f3a4b4e4cd633889eb14b5ef635b/test/RouterTest.php#L83

The client object gives further information about the client, like its IP address.

enumag commented 4 years ago

I don't need it for anything in the tests... the question is if amphp will try to access some of it's methods.

I wanted to try with a mock but I didn't get to it today - had to rebase my branch, ran into conflicts, found a bug, had to fix it, then backport, then merge and so on... I'll try next week with a mock and reopen if I run into some issues. Thanks!

enumag commented 4 years ago

Hmm ran into something else... simply calling the Router doesn't work as expected. It fails on this:

Error: Call to a member function dispatch() on null
/usr/app/vendor/amphp/http-server-router/src/Router.php:73

The Router::$routeDispatcher property is not initialized. The only assignment I found is in Router::onStart() but this method wants a Server instance. Any idea how to bypass that for tests?

I thought the router would just use FastRoute to match the URL and delegate the request to the correct RequestHandler. It wouldn't need a Server for that. But it seems it's doing some other things too...

enumag commented 4 years ago

Hmm maybe I can try with a mock like this... I'll test it on Monday.

enumag commented 4 years ago

Oh great... Segmentation Fault. 😢

enumag commented 4 years ago

Segfault seems to have been caused by ext-event so I got rid of it. My tests work now.

Another issue (admittedly unrelated to this package) is that previously I was used to wrap the entire test in a database transaction and use rollback after the test to achieve better test independence. This worked without a problem thanks to Doctrine DBAL being able to emulate nested transactions using postgres savepoints.

Now with amphp/postgres it isn't so easy because transactions work a bit differently than I'm used to (transaction being it's own object). This API does make sense to me, but I'm unsure how to achieve the rollback-after-test thing I'm used to.

Do you have some tips about this? How do you achieve independence in your own tests when it comes to database data?

kelunik commented 4 years ago

@enumag Having tests use a transaction that isn't committed at the end is definitely useful and should be supported. I currently don't have projects doing that, but I guess it makes sense to open an issue to make sure we either have an example or work to support it if the current API makes it difficult.

enumag commented 4 years ago

@kelunik It's pretty much impossible with current API. Also there is the fact that there are no nested transactions so as soon as you would use a transaction inside your app it would break.

In the end I solved it by dropping the database after each test that modified it and then recreating it using CREATE DATABASE ... WITH TEMPLATE ... which is effectively database cloning - so I just restore the original database state this way.

trowski commented 4 years ago

@enumag What do you mean it's impossible? When the Transaction object is destroyed, the transaction is rolled back. You can use "nested" transactions using createSavepoint(), releaseSavepoint(), and rollbackTo(). Please open an issue in amphp/postgres if you want to discuss or offer suggestions.

enumag commented 4 years ago

@trowski I mean that it would be very difficult to use. Normally you want the application to use transactions directly but in tests you want it to use savepoints instead because there is already an active transaction from the testcase... Anyway I don't really care. The solution I found works fine for me and feels cleaner anyway.

trowski commented 4 years ago

@enumag I see what you mean. I guess for a v2 we could consider adding beginTransaction to Transaction that uses savepoints.

The solution you're using sounds similar to how I do my testing, and something I'll probably add to the docs when I finally getting around to writing more for the DB libs.