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

Various testing improvments #1788

Open
bdsl opened this issue Jun 14, 2019 · 4 comments
Open

Various testing improvments #1788

bdsl opened this issue Jun 14, 2019 · 4 comments

Comments

@bdsl
Copy link
Contributor

bdsl commented Jun 14, 2019

I'm initially posting this to get some feedback from @muglug and others before I start on any of this or make individual issues.

I'm potentially interested in making various additions to the test pipeline for Psalm, e.g.

  1. Restore the coverage collection, and make html coverage reports visible as an aid to writing tests.
  2. Add mutation testing using Infection ( Monitor mutation coverage score of test suite #1797)
  3. Add a few end-to-end tests, that invoke psalm as a separate process and assert on the return code and output.
  4. Test the built phar file before deploying it
  5. Maybe add some tests that use big well known PHP codebases as test cases, e.g. Wordpress, Drupal, Symfony, Laravel etc.
  6. Possibly some unit testing for smaller parts of the code. Currently I think psalm tests are mostly functional and based on ValidCodeAnalysisTestTrait and InvalidCodeAnalysisTestTrait, which work by passing a string of PHP Code to the FileAnalyzer and asserting on the error message Psalm should emit, and there are relatively few tests that exercise individual functions, classes etc.
  7. Consider refactoring tests that use those two traits, so that instead of each string of PHP being a literal string within the test class, it is its own PHP or PHPT file. Possibly this would hurt performance too much to be worthwhile, but the advantage would be that IDEs would assist much more with writing this test code, and we could see at a glance how they analyse it, and if we used PHP it would be trivial to manually run Psalm with any given file. If I was to do this I think the change would be too big to review manually, so instead I would write a script to create all the files and ask @muglug to run it and commit the result.

However for a lot of this I think Travis is too limiting, as it has a 1 hour limit for each job, and it doesn't seem to have any easy way to carry artefacts between jobs, or offer artefacts for download. Instead I'd probably want to use CircleCI, since it supports artefacts, apparently will allow builds of up to 5 hours, and allows free unlimited use of up to three concurrent container instances for open source projects.

Some of these ideas could lead to a full test pipeline that would take multiple hours to run, but I think if it's well organised so that the most useful tests run first and the others only rarely fail that could be manageable.

@muglug
Copy link
Collaborator

muglug commented Jun 14, 2019

Thanks for these suggestions! I have thoughts:

Restore the coverage collection, and make html coverage reports visible as an aid to writing tests.

Yeah, will need to migrate to pcov for this (xdebug now too slow)

Add mutation testing using Infection

Open to this, though not as high a priority as other stuff on this list.

Add a few end-to-end tests, that invoke psalm as a separate process and assert on the return code and output.

Sounds good

Test the built phar file before deploying it

Definitely

Maybe add some tests that use big well known PHP codebases as test cases, e.g. Wordpress, Drupal, Symfony, Laravel etc.

Pre-release I run Psalm against a few open source projects that use it - PHPUnit, CakePHP and Monica. It’d be great to have a bash script that automates that process. It could run Psalm against the projects you mentioned, and compare the output against some sort of checked-in baseline e.g. tests/project_baselines/symfony-<commit>.xml to ensure no regressions.

Possibly some unit testing for smaller parts of the code. Currently I think psalm tests are mostly functional and based on ValidCodeAnalysisTestTrait and InvalidCodeAnalysisTestTrait, which work by passing a string of PHP Code to the FileAnalyzer and asserting on the error message Psalm should emit, and there are relatively few tests that exercise individual functions, classes etc.

There are definitely things that could be more precisely unit-tested (e.g the type reconciler) but functional tests are incredibly easy to write.

Consider refactoring tests that use those two traits, so that instead of each string of PHP being a literal string within the test class, it is its own PHP or PHPT file. Possibly this would hurt performance too much to be worthwhile, but the advantage would be that IDEs would assist much more with writing this test code, and we could see at a glance how they analyse it, and if we used PHP it would be trivial to manually run Psalm with any given file. If I was to do this I think the change would be too big to review manually, so instead I would write a script to create all the files and ask @muglug to run it and commit the result.

Not a big fan of this - I like seeing similar tests in plain text together (though some of the large test files should definitely be broken up). I doubt performance would be affected, though.

@bdsl
Copy link
Contributor Author

bdsl commented Jun 14, 2019

What do you think about using CircleCI? The artefact support and execution time seem very valuable.

@bdsl
Copy link
Contributor Author

bdsl commented Jun 16, 2019

@muglug thanks for merging the PR. It'd be good to know if you're considering fully replacing Travis with Circle — if so I can probably make some more PRs to port over the missing parts of the set up from Travis. It would mean that the deployment to the psalm/phar repo could use the exact same phar file built once and tested in https://github.com/vimeo/psalm/blob/master/bin/test-with-real-projects.sh

@muglug
Copy link
Collaborator

muglug commented Nov 6, 2020

ok now definitely considering replacing Travis with Circle, given Travis is basically going away for OS developments

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants