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

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

Merged
merged 2 commits into from
Jan 14, 2024

Conversation

llaville
Copy link
Contributor

@llaville llaville commented Jan 9, 2024

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
Copy link
Collaborator

sidz commented Jan 9, 2024

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 #176

I'm okay to merge your PR and release 3.4.1 version and after that we could merge #176 and release 3.5.0

WDYT @llaville ?

cc/ @exussum12, @kubawerlos

@llaville
Copy link
Contributor Author

llaville commented Jan 9, 2024

@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
Copy link
Collaborator

sidz commented Jan 9, 2024

@llaville yeah, that exactly what I did in #176

@llaville
Copy link
Contributor Author

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

@sidz sidz merged commit e34c2f4 into povils:master Jan 14, 2024
21 checks passed
@sidz
Copy link
Collaborator

sidz commented Jan 14, 2024

Merged.

Thanks @llaville

@llaville llaville deleted the application-version-up-to-date branch January 14, 2024 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants