Add core unit tests (#170) #183
Conversation
| * | ||
| * @link http://scm.monitorlinq.com/backend | ||
| * @copyright Copyright (c) 2016 Monitorlinq Limited | ||
| * @license http://www.monitorlinq.com/license Proprietary License |
There was a problem hiding this comment.
My mistake, will remove it and add to the PR.
| $application = new Application(); | ||
|
|
||
| $this->assertInstanceOf('\PHPSA\Application', $application); | ||
| $this->assertInstanceOf('\Symfony\Component\Console\Application', $application); |
There was a problem hiding this comment.
There was a problem hiding this comment.
Am okie to remove, was trying to protect backwards compatibility (construct function without parameters) when the construct function might get parameters in the future.
Let me know.
There was a problem hiding this comment.
I dont known, for me it's not need
It's rly simple tests that cannot give nothing, about BC I dont thing that someone will extends from \PHPSA\Application, but maybe...
| class ApplicationTest extends TestCase | ||
| { | ||
| /** | ||
| * @covers \PHPSA\Application::__construct |
There was a problem hiding this comment.
The annotation ensures that the coverage is limited to that function(s) that is/are being tested. To ensure the coverage is only measured to what is validated using assertions.
There was a problem hiding this comment.
I know what it does, I'm just not sure that we actually want to use this feature :)
There was a problem hiding this comment.
I am okey with @Covers, https://phpunit.de/manual/3.7/en/appendixes.annotations.html#appendixes.annotations.covers
This can help to avoid green code on code coverage, but it's not an important thing for me
And this @Covers helps to navigate in the code with clicking on it
|
Hi Guys, I'm having some time to continue this. Any comments on what is needed to accept the PR? BR |
|
I think if you remove the method testConstructor in ScopePointerTest and the ApplicationTest file we can merge. |
|
@ddmler Please find the additional commits removing the constructor test and support the changes merged from upstream. |
|
Thanks @DannyvdSluijs 👍 |
|
[phpsa] Branch "master" was force-pushed to |
|
branch, was interactive rebased and cherry-picked into master without
|
|
@ovr Thanks for taking the issue. Could you show the specific steps/commands used to achieve the rebase. Rebasing is still somewhat unclear to me. Next time it would like to spare you the trouble. |
|
@ovr sorry didn't see the merge commit |
@DannyvdSluijs , @ddmler write you link or your can review how to rebase in our documentation inside project Sorry, for late reply :( |
Hey!
Type: code quality
Link to issue: #170
This pull request affects the following components: (please check boxes)
In raising this pull request, I confirm the following (please check boxes):
Small description of change: Added core unit tests for IssuesCollector, ScopePointer and Application
Thanks