povils / phpmnd

PHP Magic Number Detector
MIT License
550 stars 46 forks source link

Add test to verify that `Application::VERSION` match the latest git tag #175

Closed llaville closed 8 months ago

llaville commented 8 months ago

Hello,

It's almost always the case when we use an hard coded version (with Application::VERSION) to forget to bump it before to push a new release !

To avoid in future to avoid such case, I propose this PR that add a TestCase that is run by CI only when you'll push a tag to repository.

When you push code to a branch, PHPUnit will run this command vendor/bin/phpunit --exclude-group tags

PHPUnit 9.6.15 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.2.14
Configuration: /shared/backups/forks/phpmnd/phpunit.xml.dist
Random Seed:   1704792472

..........................................                        42 / 42 (100%)

Time: 00:00.231, Memory: 18.00 MB

OK (42 tests, 72 assertions)

But when you'll push a tag to repo, PHPUnit will run all tests including this new one (i.e) :

PHPUnit 9.6.15 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.2.14
Configuration: /shared/backups/forks/phpmnd/phpunit.xml.dist
Random Seed:   1704792623

......................F....................                       43 / 43 (100%)

Time: 00:00.266, Memory: 20.00 MB

There was 1 failure:

1) Povils\PHPMND\Tests\Application\ApplicationTest::testApplicationVersionInstalled
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'phpmnd version g6b12352 by Povilas Susinskas'
+'phpmnd version 3.4.0 by Povilas Susinskas'

/shared/backups/forks/phpmnd/tests/Application/ApplicationTest.php:38

FAILURES!
Tests: 43, Assertions: 73, Failures: 1.
sidz commented 8 months ago

Thank you @llaville. This PR totaly makes sense to me as I all the time forget to update version and I also don't have permission to amend code without pull request.

As an alternative solution I could suggest to rely on composer-runtime-api v2 which will automatically fetch version from composer.lock on the consumers side. See https://github.com/povils/phpmnd/pull/176

I'm okay to merge your PR and release 3.4.1 version and after that we could merge https://github.com/povils/phpmnd/pull/176 and release 3.5.0

WDYT @llaville ?

cc/ @exussum12, @kubawerlos

llaville commented 8 months ago

@sidz I'm agree with you about using composer-runtime-api v2. I did it myself on one of my projects like (https://github.com/llaville/php-compatinfo/blob/master/src/Presentation/Console/Application.php#L61-L64)

But in this case you must set the version on Application class constructor and do not use anymore the Application::VERSION constant. Warning : The TestCase i've introduced with this PR won't work with composer-runtime-api

sidz commented 8 months ago

@llaville yeah, that exactly what I did in https://github.com/povils/phpmnd/pull/176

llaville commented 8 months ago

Any news about merging status of thid PR or not in favor of another alternative ?

sidz commented 8 months ago

Merged.

Thanks @llaville