search

MarkBaker / PHPMatrix

PHP Class for handling Matrices
MIT License
1.44k stars 13 forks source link

Improved tests #1

Closed yaroslavche closed 5 years ago

yaroslavche commented 5 years ago

Added infection/infection, phpstan/phpstan. Added FunctionsTest Fixed tests namespace Updated dev-dependecies Removed php-documentor as dev-dependency (need workaround or approve)

Tools: composer phpstan composer infection

Metrics:
         Mutation Score Indicator (MSI): 89%
         Mutation Code Coverage: 95%
         Covered Code MSI: 94%

WIP.

yaroslavche commented 5 years ago

Hi, @MarkBaker! My proposition: to set the required version php ^7.2, since 7.1 will soon be unsupported. As you can see in travis ^7.2 works fine. I should fix nightly also.

If you want to leave supported versions as it is, you can simply leave the test files and edits with these tools. Also i could can try to implement support. But personally, my opinion is that it is not worth the effort, because these versions are not supported. And if someone really need a specific old one, let them use releases =)

MarkBaker commented 5 years ago

My proposition: to set the required version php ^7.2, since 7.1 will soon be unsupported. As you can see in travis ^7.2 works fine. I should fix nightly also.

This library is used by other libraries (e.g. PHPSpreadsheet) that do still have a minimum PHP ^5.6 for the moment, so that isn't an option till the release of PHP 7.4, when I plan to switch them to PHP ^7.1, and this library can then be adjusted accordingly. At that point I'll change this to a 2.0.0 version, because there are a number of new features that I'm working on (e.g Decompositions) for that release

MarkBaker commented 5 years ago

Just out of interest, why remove the type hints?

yaroslavche commented 5 years ago

Just out of interest, why remove the type hints?

Oh, sorry, I will revert that back. I blunted something, and decided that the type-hints added along with the return type =) UPD I've remembered why: if there will be type-hint - then we can't pass array as argument.

This library is used by other libraries

Ah, ok, I understand. Seems it can be solved. Need just use dev dependecies which not requires 7.2

yaroslavche commented 5 years ago

So, seems I've completed. With composer.7.2.json 100% code coverage =) With composer.json (old packages) it is almost 100%, IDK why, research is needed. I know that my vision of writing tests is also not perfect and tests can be optimized. Also, this is not a good solution with replacing the composer.json file in .travis.yml for ^7.2. But it does work. Hope this helps and gives you some ideas for the new version =)

Screenshot_20190903_231517

MarkBaker commented 5 years ago

Sorry it's taken me a while to get back to you; but with a number of different changes in the one PR, I've been trying to separate them down to the individual changes while I've assessed them so that I can evaluate them one at a time.

MarkBaker commented 5 years ago

But thank you for your efforts, it really is appreciated. It hasn't only improved this library, but also shown me how to add infection and phpstan to some of my other libraries, a task that I had been planning but hadn't yet got round to doing.