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

behavior tests #907

Merged
merged 2 commits into from Apr 21, 2023
Merged

behavior tests #907

merged 2 commits into from Apr 21, 2023

Conversation

jhdxr
Copy link
Contributor

@jhdxr jhdxr commented Apr 20, 2023

TL;DR
This is a working example of behavior test. Consider workerman as a black box server and verify tests as clients.

As discussed in #904 , I mentioned the behavior test is not doable because pest lacks supporting for isolation process tests. However, after experiments with PhpUnit, I realized @runinseparateprocess doesn't help as well. The only programming way to kill the service itself from inside is to kill the whole process, but even for PhpUnit it still requires to do certain jobs after the test method(s). Killing the proces is considered as an error and will leads to a test failure.

This PR proposed a different way for such tests. Instead of trying to asset values on both side, we consider the server side as a black box and only pay attention to the value we sent and received as a client. To archive this, we create a new process during beforeAll/setUp (with the help of symfony's PhpProcess) and stop it during afterAll/tearDown. Thus all tests in same file/class can communicate with the server and checking the server's behavior.


p.s. a not-so-small suggestion. Tests are not just about adding more test cases. Workerman also needs to be changed/modified to enable better and more comprehensive tests. e.g. When I tried to start workerman, the only way I found is to override $argv. There is a static::$command and getArgv in Worker so I guess the author does have an intention to support such use case. However, parseCommand still only use $argv as the only input, which make $command much less useful.

@walkor walkor merged commit 8fa75ca into walkor:master Apr 21, 2023
4 checks passed
walkor added a commit that referenced this pull request Apr 21, 2023
@walkor
Copy link
Owner

walkor commented Apr 21, 2023

Tests are not just about adding more test cases. Workerman also needs to be changed/modified to enable better and more comprehensive tests. e.g. When I tried to start workerman, the only way I found is to override $argv. There is a static::$command and getArgv in Worker so I guess the author does have an intention to support such use case. However, parseCommand still only use $argv as the only input, which make $command much less useful.

That's ok. I Removed unnecessary $argv.
Thank you for your post.

@xpader
Copy link

xpader commented Apr 25, 2023

I have a suggestion before, that is let workerman can run only once like a command, and no master and childprocess, only one process, the process exit after something finished.
Consider to implement that? @walkor

@jhdxr
Copy link
Contributor Author

jhdxr commented Apr 25, 2023

I have a suggestion before, that is let workerman can run only once like a command, and no master and childprocess, only one process, the process exit after something finished.

I think what you described is how Workerman works under Windows.
However, I don't think it's a good idea for testing purpose. We should try to test in an enviroment closing to the actual production.

venus-heaven pushed a commit to venus-heaven/php-workname that referenced this pull request Mar 6, 2024
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

3 participants