search

brick / money

A money and currency library for PHP
MIT License
1.61k stars 96 forks source link

Skip intl tests if extenstion is not installed #31

Closed solodkiy closed 2 years ago

solodkiy commented 3 years ago

Resolve test errors if intl extenstion is not installed 


1) Brick\Money\Tests\MoneyTest::testFormatTo with data set #0 (array('1.23', 'USD'), 'en_US', false, '$1.23')
Error: Class 'NumberFormatter' not found

/home/doc/projects/_opensource/money/src/Money.php:686
/home/doc/projects/_opensource/money/tests/MoneyTest.php:900
BenMorel commented 3 years ago

Good point! Shouldn't we make it a dev dependency instead, though? We already have ext-pdo and ext-dom as dev dependencies.

solodkiy commented 3 years ago

We have similar skip tag in tests/ExchangeRateProvider/PDOProviderTest.php, so I did it the same way

BenMorel commented 3 years ago

Indeed, although it shouldn't have been there, it's redundant as composer will warn if the dev dependency is not installed anyway.

So we should either make all extensions required in require-dev, or mark everything as @requires in PHPUnit. I quite like to just skip tests if the extension is not installed, rather than forcing its installation, however the downside is that PHPStorm will complain in src/ that ext-* is not present in composer.json.

solodkiy commented 3 years ago

Indeed, although it shouldn't have been there, it's redundant as composer will warn if the dev dependency is not installed anyway.

There is different extensions in test and in composer (pdo_sqlite vs ext-pdo). So this case is not redundant.

BenMorel commented 3 years ago

We only test with pdo_sqlite, so it is redundant!

BenMorel commented 2 years ago

Let's do this. @solodkiy I think we should use phpunit --fail-on-skipped in the CI now then?

solodkiy commented 2 years ago

Sounds reasonable