Larastan: Increase test coverage

[szepeviktor]

Larastan's test coverage is currently under 10%
https://travis-ci.org/nunomaduro/larastan/jobs/431661574#L631

We encourage everyone to write more tests.
PR-s are welcome!

[nunomaduro]

@szepeviktor Any ideas for this? I would love some kind of integration testing suite with a laravel application specific code that should be detected by larastan.

[szepeviktor]

We already run with a vanilla laravel.
Actually only unit tests increase coverage.

[szepeviktor]

@ondrejmirtes Do you have any idea on unit-testing extensions?

[ghost]

@szepeviktor there is not only travis ci. 😎 https://scrutinizer-ci.com/g/nunomaduro/larastan/code-structure/master/code-coverage

[ondrejmirtes]

You can use LevelsTestCase from PHPStan, check out
https://github.com/phpstan/phpstan-strict-rules/tree/master/tests/Levels

You can set up the .php files (with topic names) and it will generate the
current situation on the first run. It also updates the files and passes
only if they don’t need to be updated. Although it’s an integration test,
it spawns another process and I’m not sure whether it increases code
coverage stats. But it’s a good idea to have them.

On Wed, 26 Sep 2018 at 04:40, ForzaSFerrari [e-mail]
wrote:

@szepeviktor https://github.com/szepeviktor there is not only travis
ci. 😎
https://scrutinizer-ci.com/g/nunomaduro/larastan/code-structure/master/code-coverage

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
https://github.com/nunomaduro/larastan/issues/146#issuecomment-424565536,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAGZuOptWZ9BPzdUCg3uPCpKUGsxZrG_ks5ueuk5gaJpZM4W5din
.

>

Ondřej Mirtes

[phannaly]

Has anyone worked on it? I'm happy to involve here.

[nunomaduro]

@phannaly Fell free to propose a first test, you may want to inspire yourself on the @ondrejmirtes's comment.

[phannaly]

Ahh, thank you @nunomaduro let see if I could.

[phannaly]

Hi @nunomaduro @ondrejmirtes I want to make some PR on this but I really new to this tool and need to play around a bit. It would be great if you can make a PR then I can follow you all.

[nunomaduro]

@szepeviktor We need to start working on this.

[szepeviktor]

@nunomaduro OK, we need a theory first.

[CyberiaResurrection]

@nunomaduro , @szepeviktor , et al - I've already taken a bang at this one and bounced off.

I started trying to unit test Methods\Passable and had a devil of a time generating inputs to the constructor, so tried again with Methods\Macro, and bounced. Would either of you be averse to me pulling Mockery in as a dev dependency so I can pass mocked inputs to whatever class I'm testing?

[canvural]

@CyberiaResurrection I don't think we should unit test those methods like that. Did you check how phpstan writes tests? Or any other phpstan extension. Maybe it can give some idea.

[CyberiaResurrection]

@canvural, I suggested what I did because Larastan uses phpunit and, for whatever reason, phpstan doesn't - plus I found the phpstan tests heavy going to read. The ApplicationResolver characterisation test I landed is why I'm even mentioning this.

And Mockery is a moot point - it seems to be already pulled in (cf https://github.com/nunomaduro/larastan/blob/722cfdf581cf50740dcf56924a4973551c902d7b/src/Methods/Passable.php ).

You are correct in that I haven't made my proposed approach clear, so here goes:

• I'm proposing unit-level (single class) tests _alongside_ the existing feature tests, not _instead_ of them.

• This is to further constrain defects - if (reasonably comprehensive) unit tests pass clean and one or more feature tests fail, then the defect is most likely in how two or more classes interact. If unit test coverage is not comprehensive (such as when someone like me is raising it), then (assuming those unit tests pass), the defect is unlikely to be in the unit-tested classes.

In other words, clear the underbrush out with unit tests, while also verifying the Larastan magic still keeps working with feature tests.

[szepeviktor]

@CyberiaResurrection Please see https://github.com/phpstan/phpstan/blob/master/composer.json#L38

[CyberiaResurrection]

@szepeviktor , I sit corrected. I still found grokking the phpstan tests heavy going.

[szepeviktor]

Yes, phpstan is a bit esoteric.

[CyberiaResurrection]

I was trying to break the apparent logjam - your opening comment was over 13 months ago, @szepeviktor . Good-ish now, rather than perfect never.

[ondrejmirtes]

@CyberiaResurrection Can you elaborate on what you don't like about PHPStan tests?

[CyberiaResurrection]

@ondrejmirtes , more a style and approach that is different enough to what I'm used to to trip me up. And, as @szepeviktor said, it's somewhat esoteric - so I'm trying to nibble around the edges while I start trying to understand things.

[szepeviktor]

As per my previous incidents with PHPStan, the learning curve takes 1 advise from @ondrejmirtes and half an hour :)

[CyberiaResurrection]

I'll have to sleep on it - had a long, tiring day.

[ondrejmirtes]

I'd need a more specific feedback :) I think that rule tests are straightforward. Type-inferring tests aren't easy in 3rd party code (like Larastan) right now, but dev-master improves it a bit.

Right now for type-inferring tests (for example to test dynamic return type extensions), you need to write source code samples and try to produce an error by PHPStan to see if the type inferring and checks actually see the correct thing.

See how phpstan-doctrine does it:

• https://github.com/phpstan/phpstan-doctrine/blob/master/tests/DoctrineIntegration/ORM/EntityManagerIntegrationTest.php
• https://github.com/phpstan/phpstan-doctrine/blob/master/tests/DoctrineIntegration/ORM/data/entityManagerDynamicReturn.php
• https://github.com/phpstan/phpstan-doctrine/blob/master/tests/DoctrineIntegration/ORM/data/entityManagerDynamicReturn-2.json

If you run this test case and you don't have the JSON file at all or it's not up to date, the test case will generate/update it for you, you don't have to write it by hand.