Structure tests #60
Structure tests #60
Conversation
…r running the other tests, need to find out)
…ck function isn't called and also cover the message and name function in a test
…est and also test the custom database connections config
…a test and update the valid check test with a correct mock result
…essage function is called before the check function
…reSet in a test and prevent an error if the message function is called before the check function
…reUpToDate check in a test and prevent an error if the message function is called before the check function. Also move the test to the directory matching the namespace of the check.
… a test and prevent an error if the message function is called before the check function
… we have a good way to do that
|
Directory and file based tests could also be tested using |
|
I have created a test for the PhpExtensionsAreInstalled check, this doesn't cover the whole check because I'm waiting on #61 |
…rt (Waiting on input from #61 ) and prevent an error if the message function is called before the check function.
…essage function is used before the check function.
There was a problem hiding this comment.
I think the usage of vfsStream is good. But after having another look at the changes in general, I noticed two things which are commented below.
One test that could receive some more test attention is ServersArePingable, but to be honest, I'm unsure how to cover it without performing actual requests in its current state. It would probably require a rewrite to utilize a PingFactory and stubs or something like that.
tests/SystemFunctionsTest.php
Outdated
| public function it_find_an_available_function() | ||
| { | ||
| $systemFunctions = new SystemFunctions(); | ||
| $this->assertFalse($systemFunctions->isFunctionAvailable('print')); |
There was a problem hiding this comment.
The assert does not match the function description, I think.
There was a problem hiding this comment.
Good point, the test was incorrect, I fixed it in f3e3815
… (With thanks to Namoshek for the feedback)
|
@Namoshek thanks for all the feedback, my idea with this PR was to add test and structure but not rewriting existing tests. I feel the problem you describe with the ServersArePingable check and the test for that class. Because it already exist I didn't want to change it yet, and I'm also not sure yet what is the best approach for that check. |
|
Alright, that sounds reasonable. I'll see if I can improve the |
|
I have merged master in this branch and resolve the merge conflict for composer.json Is there anything else I need to do? |
|
@tvbeek sorry for ignoring the pr so long..I will look at this in depth later today, as the PR is quite big :) |
|
@mpociot that is nice, just let me know if something isn't clear or need to be changed. |
|
@mpociot while cleaning up my GitHub I found this PR. Do you want to take a look or should I just remove it? (Both is fine for me) |
|
Closed because no interaction. |
After #39 I have been thinking about structuring the tests. In this PR are new test, structure changes and small changes in checks to prevent failures.
What I have done:
@groupannotations to the tessSome questions I have:
@group routes) Is this wanted or not? With the current size of the project I wasn't very sure if it wanted to add.Todo:
Add a test for the PhpExtensionsAreInstalled check, this is the only check without tests so I should like to also add that one.I should really like to get some feedback on the work that is done. Is something missing? Something done that isn't wanted? Do you see something strange in the code?