search

giggsey / libphonenumber-for-php

PHP version of Google's phone number handling library
https://giggsey.com/libphonenumber/
Apache License 2.0
4.76k stars 468 forks source link

use absolute path to find data #583

Closed tenzap closed 1 year ago

tenzap commented 1 year ago

When the tests folder is not next to the src folder, about 200 tests will fail. To workaround this, use absolute path instead of relative paths to set the data dir.

Hopefully I didn't miss some files, but the tests with this change.

giggsey commented 1 year ago

I'm not sure which PHP version added class constant expressions, but it's failing with 5.3, 5.4, 5.5 (the others aborted due to the failures).

Whilst the change is minor enough, I'm not convinced it needs doing. The tests run fine if you clone the project normally.

tenzap commented 1 year ago

I don't understand what you mean with class constant expression.

Anyway, the objective is to run the tests on the library as it is installed on the OS (Debian). In that scenario the tests folder is not installed with it but is somewhere else. One of the objective is to check that the library functions as expected once it is installed at its final location.

If the tests dir is not in a folder next to the install dir, some tests will fail because they can't find the data.

If you have better way to fix that issue, it would be highly appreciated to have it in the code.

giggsey commented 1 year ago

https://www.php.net/manual/en/language.oop5.constants.php

I'll leave this PR open for now, and might have a go at fixing it within the next few weeks.

tenzap commented 1 year ago

I'm not sure which PHP version added class constant expressions, but it's failing with 5.3, 5.4, 5.5 (the others aborted due to the failures).

5.6 maybe? https://stackoverflow.com/a/36693544/15401262

giggsey commented 1 year ago

I've opened #584 with an alternative way of loading the data files without having to add horrible looking defines() and polluting the global namespace.

Could you take a look and see if it covers your use case?

tenzap commented 1 year ago

Thank you. Tests are successful with #584.