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

fix: config type breaking tests #736

Merged
merged 1 commit into from
Jul 9, 2024
Merged

Conversation

millnut
Copy link
Member

@millnut millnut commented Jun 28, 2024

Once localgovdrupal/drupal-container#33 and the other drupal-container PRs are merged and a new docker image is created for each PHP version, this PR will fix the error below which is reported in tests across ~21 modules.

1) Drupal\Tests\localgov_content_lock\Functional\ContentLockTest::testContentLockConfiguration
Drupal\Core\Config\Schema\SchemaIncompleteException: Schema errors for system.cron with the following errors: system.cron:logging variable type is integer but applied schema class is Drupal\Core\TypedData\Plugin\DataType\BooleanData

/app/web/core/lib/Drupal/Core/Config/Development/ConfigSchemaChecker.php:98
/app/web/core/lib/Drupal/Component/EventDispatcher/ContainerAwareEventDispatcher.php:111
/app/web/core/lib/Drupal/Core/Config/Config.php:230
/app/web/core/lib/Drupal/Core/Config/ConfigInstaller.php:396
/app/web/core/lib/Drupal/Core/Config/ConfigInstaller.php:149
/app/web/core/lib/Drupal/Core/ProxyClass/Config/ConfigInstaller.php:75
/app/web/core/lib/Drupal/Core/Extension/ModuleInstaller.php:326
/app/web/core/lib/Drupal/Core/ProxyClass/Extension/ModuleInstaller.php:83
/app/web/core/includes/install.inc:567
/app/web/core/includes/install.core.inc:1125
/app/web/core/includes/install.core.inc:695
/app/web/core/includes/install.core.inc:572
/app/web/core/includes/install.core.inc:122
/app/web/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php:315
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:570
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:370
/app/web/profiles/contrib/localgov/modules/localgov_content_lock/tests/src/Functional/ContentLockTest.php:33
/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:729

@ekes
Copy link
Member

ekes commented Jul 8, 2024

So this creates the error

1) Drupal\Tests\localgov\Functional\LocalGovProfileTest::testLocalGovDrupalProfile
Failed asserting that 'The following reasons prevent the modules from being uninstalled: The 'LocalGov Drupal' install profile requires 'LocalGov Core'; The install profile 'LocalGov Drupal' is providing the following module(s): localgov_login_redirect' contains "module is required".

But understand this is fixing paratest that's hiding this error?

If so happy to approve merge, it is also only a type switch in config, so we can get on with fixing things.

@millnut
Copy link
Member Author

millnut commented Jul 8, 2024

Yep paratest seems to only handle a few error returns from phpunit, the one hiding the errors is #738 as I don't think paratest can handle child processes failing so you get the weird PHP Fatal error: Uncaught AssertionError: assert(count($nodes) === 1) error.

Ideally I'd like phpunit on each module and keep paratest for the project repo only but that is probably a separate discussion.

Copy link
Member

@ekes ekes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we should get this committed so we can see what the errors from phpunit actually are.

@finnlewis finnlewis merged commit 9021abb into 3.x Jul 9, 2024
6 of 8 checks passed
@finnlewis finnlewis deleted the fix/3.x/fix-config-breaking-tests branch July 9, 2024 11:11
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.

None yet

3 participants