-
-
Notifications
You must be signed in to change notification settings - Fork 825
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
mixin/scan-classes - Don't scan extension-tests automatically #26157
Conversation
(Standard links)
|
mixin/scan-classes@1/mixin.php
Outdated
* | ||
* To minimize unintended activations, this only loads Civi interfaces. It skips other interfaces. | ||
* | ||
* @mixinName scan-classes | ||
* @mixinVersion 1.0.0 | ||
* @mixinVersion 1.0.1 |
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'd probably go for:
* @mixinVersion 1.0.1 | |
* @mixinVersion 1.1.0 |
The change is in awkward spot. Formally, it's a BC-break (so it should be v2.0.0). But in practice, I don't think anyone actually has a live use for this edge-case (so v2.0.0 would be too heavy-handed). It's a little messed-up no matter what one does.
To my eye, 1.1.0
signals that there was a non-trivial change while also signaling that it's safe for 99% use-cases.
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.
OK done - didn't spot that I could have just committed this so pushed in
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.
Aside: I think maybe that change wasn't actually pushed to Github? But it's small, so I pushed an equivalent update.
…fere with phpunit runner).
Since this file has both implicit test-coverage (via |
Port of civicrm/civicrm-core#26157 Bug: T335933 Change-Id: I2665fd5292be90f921c012d6f5700a7045c054ee
Overview
Stop running class scanner in test directories
Before
When using the test frame work test directories were scanned for relevant classes
After
Test directories no longer scanned
Technical Details
The scanning was causing an edge case bug. If the scanner runs during bootstrap (ie because civicrm_initialize is called from the bootstrap file) then the classes are loaded at that point. Later when phpunit loads the test classes it does a scan and treates 'new_classes' as new test classes - but as these are already scanned they are not added to the list.
While rather edge this variant (tests silently not running) is worse than any variant of tests noisily failing.
Comments