kitodo / kitodo-presentation

Kitodo.Presentation is a feature-rich framework for building a METS- or IIIF-based digital library. It is part of the Kitodo Digital Library Suite.
https://kitodo.github.io/kitodo-presentation/
GNU General Public License v3.0
39 stars 45 forks source link

[BUG] Unit Tests fail in TYPO3 v11 #1348

Closed sebastian-meyer closed 1 month ago

sebastian-meyer commented 1 month ago

Hi Sebastian and Markus. Your recent changes fail unit tests in my branch (php 7.4, typo3 v11). Any ideas?

There was 1 error:

1) Kitodo\Dlf\Tests\Unit\Validation\SaxonXslToSvrlValidatorTest::testValidation
InvalidArgumentException: Saxon JAR file not found.

/home/runner/work/kitodo-presentation/kitodo-presentation/Classes/Validation/SaxonXslToSvrlValidator.php:47
/home/runner/work/kitodo-presentation/kitodo-presentation/Tests/Unit/Validation/SaxonXslToSvrlValidatorTest.php:67
phpvfscomposer:///home/runner/work/kitodo-presentation/kitodo-presentation/vendor/phpunit/phpunit/phpunit:106
/home/runner/work/kitodo-presentation/kitodo-presentation/vendor/bin/phpunit:118

--

There was 1 failure:

1) Kitodo\Dlf\Tests\Unit\Validation\SaxonXslToSvrlValidatorTest::testXslFileNotFound
Failed asserting that exception message 'Saxon JAR file not found.' contains 'XSL Schematron file not found.'.

phpvfscomposer:///home/runner/work/kitodo-presentation/kitodo-presentation/vendor/phpunit/phpunit/phpunit:106
/home/runner/work/kitodo-presentation/kitodo-presentation/vendor/bin/phpunit:118

Originally posted by @thomaslow in https://github.com/kitodo/kitodo-presentation/issues/1313#issuecomment-2371180132

sebastian-meyer commented 1 month ago

@thomaslow The problem seem to be the Typo3 function GeneralUtility::getFileAbsFileName https://github.com/thomaslow/kitodo-presentation/blob/6363217ab3c7c0a1863e45dfe75de2ebe16b7949/Classes/Validation/SaxonXslToSvrlValidator.php#L43, cause with extension path EXT:dlf/Tests/Fixtures/Format/alto.xml parameter the resolved path is not returned when running with the updated typo3/testing-framework. The behavior of resolving extension paths may have changed in the new version.

Originally posted by @markusweigelt in https://github.com/kitodo/kitodo-presentation/issues/1313#issuecomment-2371565998

sebastian-meyer commented 1 month ago

@thomaslow i run the tests again with typo3/testing-framework 6.16.9, which succeeds, and with 7.1.0 I can reproduce the problem. I'll take a look at it tomorrow when I get a chance.

Originally posted by @markusweigelt in https://github.com/kitodo/kitodo-presentation/issues/1313#issuecomment-2371677049

sebastian-meyer commented 1 month ago

@markusweigelt I followed the empty string of GeneralUtility::getFileAbsFileName to the Typo3 PackageManager->getActivePackages(). It seems the package key EXT:dlf can not be resolved, because the package is not known at all when running the unit test.

The difference between testing-framework v6 and v7 seems to be in the UnitTestsBootstrap.php. They set composerMode = true, which prevents the PackageManager from scanning for extensions. Removing this check for composer mode loads all package paths including dlf and the unit test works.

I'm not sure how to fix this. I see two options:

Originally posted by @thomaslow in https://github.com/kitodo/kitodo-presentation/issues/1313#issuecomment-2371889499

sebastian-meyer commented 1 month ago

We copied Build/Test/runTests.sh a while ago from TYPO3, maybe we need to update this script in order to correctly run the tests in composerMode? The script seems really old anyways, referencing a lot of outdated versions (i. e. for MySQL) and missing current versions (like PHP 8.3).

markusweigelt commented 1 month ago

In the source code https://github.com/TYPO3/testing-framework/blob/7/Resources/Core/Build/UnitTestsBootstrap.php, composerMode is set by the environment variable TYPO3_COMPOSER_MODE. It might be enough to set this to false for running the unit tests.

The runTests.sh https://github.com/TYPO3/typo3/blob/12.4/Build/Scripts/runTests.sh was also updated, but I think fixing the issue won't have any impact. However, it would certainly be worth a try. Maybe we could possibly store one for each version, then we wouldn't need to make any adjustments in the best-case scenario.

Moving to functional tests is an option to solve the problem, but it might be confusing because these tests are unit tests. That's why I prefer using absolute paths if the changes above don't work or there is an objection is to use the constant. ;)

thomaslow commented 1 month ago

I don't think runTests.sh is specifically involved in this problem. In the end, phpunit is started. Running phpunit directly on a console causes the same test failures.

We could set TYPO3_COMPOSER_MODE to false. There does not seem to be a whole lot of references to Environment::isComposerMode, so there might not be a lot of implications for this. However, it feels wrong to me, given that we install everything with composer and clearly use the composer mode. Some Typo3 code specifically not uses GeneralUtility::getFileAbsFileName in composer mode as well, see Example.

Another option might be to create a mock object of the PackageManager and manually register the dlf package with its root packagePath. Typo3 internal unit tests seem to heavily rely on mocking various required objects. For example, the PackageManagerTest::setUp().

markusweigelt commented 1 month ago

@thomaslow Thank you for the investigation. I have added a PR that solves the problem using an absolute path. The function Environment::getExtensionsPath is used to keep the path as generic as possible.