Add getter to check if registry is enabled#54
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a public getter for the GenericModel registry toggle so callers (especially tests) can check/restore the previous registry state after temporarily disabling it.
Changes:
- Introduces
GenericModel::isRegistryEnabled()to expose the current value of the internal registry flag.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| $this->assertTrue(GenericModel::isRegistryEnabled()); | ||
| GenericModel::disableRegistry(); | ||
| $this->assertFalse(GenericModel::isRegistryEnabled()); | ||
| GenericModel::enableRegistry(); | ||
| $this->assertTrue(GenericModel::isRegistryEnabled()); |
There was a problem hiding this comment.
This test assumes the registry is enabled at the start and also risks leaving the global static registry state modified if an assertion fails after disableRegistry(). Since other tests in the suite call disableRegistry() without re-enabling it (e.g., TestDriverTest::testUpdate()), this can make the test order-dependent/flaky. Consider capturing the initial state with GenericModel::isRegistryEnabled() and restoring it in a try/finally (or tearDown()), and avoid asserting a hard-coded initial true unless you explicitly set it first.
| $this->assertTrue(GenericModel::isRegistryEnabled()); | |
| GenericModel::disableRegistry(); | |
| $this->assertFalse(GenericModel::isRegistryEnabled()); | |
| GenericModel::enableRegistry(); | |
| $this->assertTrue(GenericModel::isRegistryEnabled()); | |
| $initialState = GenericModel::isRegistryEnabled(); | |
| try { | |
| // Ensure registry is enabled so we can test disabling and re-enabling it | |
| if (!$initialState) { | |
| GenericModel::enableRegistry(); | |
| } | |
| $this->assertTrue(GenericModel::isRegistryEnabled()); | |
| GenericModel::disableRegistry(); | |
| $this->assertFalse(GenericModel::isRegistryEnabled()); | |
| GenericModel::enableRegistry(); | |
| $this->assertTrue(GenericModel::isRegistryEnabled()); | |
| } finally { | |
| // Restore original registry state to avoid leaking global changes | |
| if ($initialState !== GenericModel::isRegistryEnabled()) { | |
| if ($initialState) { | |
| GenericModel::enableRegistry(); | |
| } else { | |
| GenericModel::disableRegistry(); | |
| } | |
| } | |
| } |
This could be useful to ensure you return the registry to its previous state after temporarily disabling it or to verify that your code doesn't accidentally leave the registry disabled in a test.