tests/events/*.php - Allow optional runtime enforcement #28723
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
|
🤖 Thank you for contributing to CiviCRM! ❤️ We will need to test and review this PR. 👷 Introduction for new contributors...
Quick links for reviewers...
|
This variant of hook_civicrm_links has multiple compliance issues. It raises so many red-flags that it makes the site unbrowseable and obscures any useful signal about the other hooks. Skip it until someone can take a full look.
ebe0603 to
d19c9bb
Compare
artfulrobot
reviewed
Dec 20, 2023
Comment on lines
+77
to
+88
| public static function assertNotEmpty($actual, string $message = ''): void { | ||
| if (empty($actual)) { | ||
| $comment = 'Value should not be empty.'; | ||
| static::fail(trim("$message\n$comment")); | ||
| } | ||
| } | ||
|
|
||
| public static function assertTrue($condition, string $message = ''): void { | ||
| if ($condition !== TRUE) { | ||
| $comment = 'Value should be TRUE.'; | ||
| static::fail(trim("$message\n$comment")); | ||
| } |
There was a problem hiding this comment.
I'm guessing these are just what we need right now and no more, but it feels weird to have assertNotEmpty and not assertEmpty; assertTrue and not assertFalse.
There was a problem hiding this comment.
@artfulrobot Yup, exactly. I'm sure the vocabulary of assertions could be tuned-up more going forward.
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
The folder
tests/events/(created by #21615) allows one to declare (and enforce) the signature for a hook. However, it only supports enforcement within unit-tests. This PR (optionally) expands enforcement to runtime.Before
Assert).After
event_check, then event-checks are also enforced at runtime. (Any non-conformant hook... on any page-request... will raise an exception.)Assertare not available at runtime.) Instead, the checks throw their own exception.Comments
I think it would make sense to enable this by default on local dev builds. I placed it next to another dev-tool (
tools/extensions/phpstorm).