[Ref] Remove function parameter only used from test #20459
Conversation
|
(Standard links)
|
[Ref] Remove function parameter only used from test The test passes a function parameter to rebuild triggers but the only other call to this function in the civi-verse doesn't. We shouldn't make this function more complex for just the test. Note the function also uses the force param which I didn't move to the test call - it's possible when the tests all run together I'll need to force cache clearing but this seems like a dumb way
|
|
||
| $schema = new CRM_Logging_Schema(); | ||
| $schema->fixSchemaDifferencesForAll(TRUE); | ||
| $schema->fixSchemaDifferencesForAll(); | ||
| Civi::service('sql_triggers')->rebuild(); |
There was a problem hiding this comment.
I think the first run of the test failed because it needs rebuild($tables=NULL, $force=TRUE) to be equivalent to the previous call to triggerRebuild($tables=NULL, $force=TRUE); If that works, then +1 for merging.
Alternatively - you might be able to swap the two lines with just a call to $schema->fixSchemaDifferences() or $schema->fixSchemaDifferences(TRUE), since that function basically does these two lines.
There was a problem hiding this comment.
Yeah - I'm trying to get rid of that $force being passed in because I don't think we should be offering it as a parameter purely to support this test - might take a couple of rounds fight with jenkins to get there
There was a problem hiding this comment.
Ok - unsetting that static worked
Overview
[Ref] Remove function parameter only used from test
Before
After
$rebuildTriggers parameter removed
Technical Details
The test passes a function parameter to rebuild triggers but the only other
call to this function in the civi-verse doesn't. We shouldn't make this function
more complex for just the test.
Comments
There could be some test caching issues that need re-solving - will see what jenkins says - as this was obviously at least in part there for that