(dev/core#4088) ClassScanner - Move test registration #25415
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
(Standard links)
|
CRM/Utils/Hook.php
Outdated
| @@ -309,6 +309,10 @@ public function runHooks( | |||
| * @param $moduleList | |||
| */ | |||
| public function requireCiviModules(&$moduleList) { | |||
| foreach ($GLOBALS['CIVICRM_👻_MODULES'] ?? [] as $prefix) { | |||
There was a problem hiding this comment.
It makes it less likely that downstream will use it in unintended ways... because the name is elusive...
There was a problem hiding this comment.
Or do you think this variable might come back to haunt us?
There was a problem hiding this comment.
@totten I'm a bit on the fence - but I wouldn't want to type it into a CLI for any reason
There was a problem hiding this comment.
It makes it less likely that downstream will use it in unintended ways... because the name is elusive...
or perhaps the name is 'ghostly'
Before: The `ClassScanner` includes a special rule to load some mocks/examples from `tests/phpunit/` which are needed for some core tests. But (reportedly) it will load even when running other test-suites. After: The special rule has moved to the `bootstrap.php` for core tests. It should be inert when running other test-suites. Technical Details: I believe the reason why the special rule was originally embedded into `ClassScanner` was that `ClassScanner` has special place in bootstrap/system-lifecycle. To get around this, the patch adds `CIVICRM_FORCE_MODULES` as a way to pre-register some hook listeners. (To wit: `civitest` is a "force-enabled module" defined by `bootstrap.php`. It's always-on; it can participate in special/pre-boot hooks; and it doesn't present as a configurable extension. You might also call it a "ghost module"...)
7dd634c to
65abec3
Compare
wmfgerrit
referenced
this pull request
in wikimedia/wikimedia-fundraising-crm
Feb 16, 2023
Note we have some recent patches that are included. These are mostly ones merged to civicrm-core since forking for 5.58. One is in 5.58 now so is listed for completeness & 2 minor ones are not yet merged upstream |pr|status|notes| |[25565](https://github.com/civicrm/civicrm-core/pull/25565)|merged to master(5.60)|Supports T327963 - activity types for thank you| |[25516](https://github.com/civicrm/civicrm-core/pull/25516)|merged to master(5.60)|Helps import - filter imported searches| |[25527](https://github.com/civicrm/civicrm-core/pull/25527)|not yet merged| but performance fix for dedupe searches (the name & address ones Sandra is doing) |[25433](https://github.com/civicrm/civicrm-core/pull/25433)|merged to rc (5.59)|fix for search kit display| |[25482](https://github.com/civicrm/civicrm-core/pull/25482)|Already in 5.58.1|Fix for search kit display| |[25418](https://github.com/civicrm/civicrm-core/pull/25418)|Merged to rc (5.59)|Permanent imap timout fix| |[25226](https://github.com/civicrm/civicrm-core/pull/25226)|Merged to rc (5.59)|ReportInstance api - in our install| |[25568](https://github.com/civicrm/civicrm-core/pull/25568)|not yet merged|Helps with new import code, reduces chance of crash| |[24515](https://github.com/civicrm/civicrm-core/pull/25415)|Merged to rc (5.59)| Fix to stop CI from breaking, replaces our hack| List of cherry picked commits from the above 18d82af27b9 (HEAD -> master) Filter expired searches from search kit results a75b98cb47a Add test to is_current on SavedSearch, fix e4d69230ad1 Increase timeout on imap 89ab855619f Do not crash the whole SearchKit UI if one entity fails 0142e5dd18c Fix performance on deciding which tables to query cd710af9733 dev/core#4117 Add is_current to UserJob, Search 2fa17fb88ae Make activity_type_id available to links, test 2d2bd93cda8 (dev/core#4088) ClassScanner - Move unit-test registration e3e669d4556 Add Report Instance apiv4 Bug: T329681 Change-Id: I839d1bbfabfe3558094f7445a4281f237b609fed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
This loosens the interdependence between
Civi\Core\ClassScannerandtests/phpunit/CRM/*/WorkflowMessage.See also: https://lab.civicrm.org/dev/core/-/issues/4088
Before
The
ClassScannerincludes a special rule to load some mocks/examples fromtests/phpunit/which are needed for some core tests. But (reportedly) it will load even when running other test-suites.After
The special rule has moved to the
bootstrap.phpfor core tests. It should be inert when running other test-suites.Technical Details
I believe the reason why the special rule was originally embedded into
ClassScannerwas thatClassScannerhas special place in bootstrap/system-lifecycle. To get around this, the patch addsCIVICRM_FORCE_MODULESas a way to pre-register some hook listeners. (To wit:civitestis a "force-enabled module" defined bybootstrap.php. It's always-on; it can participate in special/pre-boot hooks; and it doesn't present as a configurable extension. You might also call it a "ghost module"...)