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

[EventLoop] Libev implementation #54

Merged
merged 34 commits into from
Nov 15, 2012
Merged

[EventLoop] Libev implementation #54

merged 34 commits into from
Nov 15, 2012

Conversation

igorw
Copy link
Contributor

@igorw igorw commented Jul 14, 2012

No description provided.

igorw added 10 commits July 5, 2012 02:07
* master:
  [Stream] Rename (ReadableStream|WritableStream){ => Interface}
  Update composer.json minimum-stability to dev
  Update composer.lock
  [Http] Add example to test for memory leaks
  [Stream] Rewrite parallel-download example to use Stream\Stream
  [Stream] Move most of Socket\Connection logic to Stream\Stream, generic ftw
  [Socket] Connection should forward drain events from the buffer
  [Socket] Suppress notice when binding to a port, fixes #45
  [Stream] Add ReadableStream::isReadable and WritableStream::isWritable
  [Stream] Ensure Buffer closes after buffered end write
@igorw
Copy link
Contributor Author

igorw commented Jul 14, 2012

Ping @m4rw3r


public function addTimer($interval, $callback)
{
return $this->createTimer($interval, $callback, 0);
Copy link

Choose a reason for hiding this comment

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

Maybe better use a const, instead of a magic value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's a "boolean integer" :)

Copy link

Choose a reason for hiding this comment

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

;-)

* master: (87 commits)
  [Http] Add tests for Request::expectsContinue()
  [Http] Add a test for Response::writeContinue()
  [Http] Add support for HTTP/1.1 continue status codes
  Update CHANGELOG
  handle write errors in Stream Buffer
  Make Espresso\Stack work with invokables
  Replace calls to array_merge with array_values
  Remove useless chr calls in DNS parser
  CHANGELOG for v0.2.0
  Fix some constraints to not use 0.2.*
  Update README and sub-composer.json files to point to 0.2.*
  Bump branch-alias to 0.2
  Update composer.lock
  Update git-subsplit script
  Add dns component to subtree split script
  [dns] Add todos and references to README
  [dns] Add RFCs as documentation
  [dns] Add todos for more robust resolver
  [dns] Parser test case with two answers
  [dns] Parse all answers from response, not just first one
  ...
@igorw igorw closed this Oct 17, 2012
@igorw igorw reopened this Oct 17, 2012
@igorw
Copy link
Contributor Author

igorw commented Oct 17, 2012

Argh, closed the wrong one.

* master:
  Add DNS timeouts to CHANGELOG
  [Dns] Minor CS adjustments
  [Dns] improved error message
  [Dns] Handle timeouts in Executor
  Add dns component to replace statements in composer.json
  Update lock file
  Drop the minimum-stability requirement
  Drop espresso from core, closes #78
  Remove now obsolete feof call
  may report Wrong parameters on line 93,
  Update changelog for v0.2.1
  Emit correct exception when trying to write to closed stream
@igorw
Copy link
Contributor Author

igorw commented Oct 17, 2012

I've disabled the LibEventLoop tests for now, as they are causing errors on travis. I'll make another WIP PR for adding them back.

This is now ready for merge.

* master:
  Fix branch alias in composer.json
  Add a note about reactor pattern and inspirations from other languages
  Add a description of what react is to the README

Conflicts:
	composer.lock
* master: (47 commits)
  Add http-client subtree split goodness
  Fix typo in http-client component composer.json
  Prepare 0.2.2 release
  Add http-client to the CHANGELOG
  [http-client] Cosmetic fixes to README and tests
  typo
  [http client] reformatted readme
  [http client] depends on react/dns
  [http client] remove async name resolving from TODO
  codes style
  [http client] use the dns component for async name resolving
  [http client] updated composer.json
  [http client] improved README
  [http client] added composer.json
  [http client] added readme and example
  [http client] React\Http\Client -> React\HttpClient
  [http client] clear buffer once it's not needed anymore
  [http client] code style
  [http client] php5.3 fix
  [http client] init state
  ...

Conflicts:
	CHANGELOG.md
	composer.lock
@igorw
Copy link
Contributor Author

igorw commented Nov 3, 2012

One of the travis builds failed because of github. It is actually good to go.

@cboden
Copy link
Member

cboden commented Nov 11, 2012

Can't merge yet. If the user hasn't installed php-libev the unit tests die (not fail, PHP Fatal Error the whole suite). LibEv should be an option but not required for React.

I tried to update the LoopTest file with factory methods for each loop type, returning a mock that called skipTest if the corresponding libraries aren't available...but apparently my PHPUnit foo is very rusty and I failed.

Also, the LibEvent tests are commented out...it seems to be due to timer issues but we should have some tests for them, even if not timer tests.

@nrk
Copy link
Member

nrk commented Nov 11, 2012

Honestly, I'd make LoopTest abstract and create StreamSelectLoopTest, LibEventLoopTest and LibEvLoopTest extending it rather than having a single LoopTest class using @dataProvider.

This approach should also make it possible to leverage a new annotation added in PHPUnit 3.7 that is indeed useful in this case: @requires. Just sticking a @requires extension libev should be enough to make the test suite skip tests that require LibEv, same with libevent.

* master:
  Fix CHANGELOG indentation
  Fix branch aliases of subtree splits to be 0.2-dev
  [Http] Correctly emit end event for Request
  [Http] Correctly emit close event in response
  Add guzzle deps update to CHANGELOG
  Update guzzle dependencies to 3.0.*
  [Http] Make response forward drain events from the connection
  Rename $resolver to $dns in examples
  Add new test-memory script that only tests the loop

Conflicts:
	CHANGELOG.md
	composer.lock
@igorw
Copy link
Contributor Author

igorw commented Nov 12, 2012

Adding it as a require-dev. And imo duplicating the test cases is not worth it.

@nrk
Copy link
Member

nrk commented Nov 13, 2012

@igorw you wouldn't really duplicate most of the test code since it'd be inherited from the abstract LoopTest class, moreover you'd be free to easily add test cases specific to each loop implementation if needed in the future, but oh well anything is fine to me as long as it works.

@igorw
Copy link
Contributor Author

igorw commented Nov 13, 2012

@nrk I'm pretty sure that test methods on abstract classes are not executed. At least this was the case last time I tried.

@nrk
Copy link
Member

nrk commented Nov 13, 2012

@igorw test methods defined in abstract classes are executed on the inheriting test classes, I use this "feature" all the time (at least since PHPUnit 3.4).

@igorw
Copy link
Contributor Author

igorw commented Nov 13, 2012

Seems to be working fine, updated. :)

@cboden cboden merged commit 334be69 into master Nov 15, 2012
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

4 participants