-
Notifications
You must be signed in to change notification settings - Fork 14
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
Issue #638: Fix all our tests #639
Conversation
justafish
commented
Aug 8, 2024
•
edited
Loading
edited
- Replace sserbin/twig-linter with vincentlanglet/twig-cs-fixer/ - sserbin/twig-linter depends on an old version of symfony/console and core is adopting this new package at some point https://www.drupal.org/project/drupal/issues/3284817 - it also has a much better extension mechanism which will help if something like Fix linting for Storybook #615 comes up again
- Update Pantheon settings for Drupal 11
- Update PHPUnit configurations and tests for Drupal 11
- Fix mysterious yarn issues with DDEV
You've buried the lede here with "Drupal 11 compatibility"! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should consider this a BC break, and bump Drainpipe to v4? In particular, if the twig linter has any sort of different rules, or if teams were expecting the twig linter in other custom tasks.
ddev exec npm i -g yarn@1.22.1 | ||
ddev exec 'ln -s $(npm root -g)/yarn/bin/yarn /usr/local/bin/yarn' | ||
ddev exec yarn init -y | ||
ddev exec yarn add focus-trap@^6.7.3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's this package for? Is it just an easy way to do a test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's testing whether the package import works correctly when bundling
|
||
include: | ||
- php-version: 8.1 | ||
drupal-version: ":^10" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice.
$this->assertEquals(REQUEST_TIME, $user->getCreatedTime(), 'Creating a user sets default "created" timestamp.'); | ||
$this->assertEquals(REQUEST_TIME, $user->getChangedTime(), 'Creating a user sets default "changed" timestamp.'); | ||
$this->assertEquals($request_time, $user->getCreatedTime(), 'Creating a user sets default "created" timestamp.'); | ||
$this->assertEquals($request_time, $user->getChangedTime(), 'Creating a user sets default "changed" timestamp.'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👏
@deviantintegral I've made some tweaks to make it compatible with both D10 and D11. I don't think we need to bump to v4 as the twig linter wasn't configurable previously. |