04: Config facade - #4
Conversation
…tate - Assert Config_Exception is catchable as RuntimeException; nothing covered that inheritance, so dropping it would have left the suite green. - Reset Config in setUp() as well as tearDown(), so the tests asserting on default state no longer depend on class execution order. - Note that the container fixture inherits DI52's class_exists() fallback in has(). - Drop a README pointer to a section that does not exist yet.
| /** | ||
| * @since 1.0.0 | ||
| * | ||
| * @param string $version Host plugin version. | ||
| * | ||
| * @return void | ||
| */ | ||
| public static function set_version( string $version ): void { | ||
| self::$version = $version; | ||
| } | ||
|
|
||
| /** | ||
| * @since 1.0.0 | ||
| * | ||
| * @return string | ||
| */ | ||
| public static function get_version(): string { | ||
| return self::$version; | ||
| } |
There was a problem hiding this comment.
I think this accidentally was left in the engineering plan after some edits. It shouldn't be needed or used anywhere, right?
The only time we would ever care about the plugin version would be for weird cases like ProPanel v3.0 and that shouldn't be a concern for this library. We should be able to remove any version-related code from the Config object.
| /** | ||
| * Reset all static state. Test seam. | ||
| * | ||
| * @since 1.0.0 | ||
| * | ||
| * @return void | ||
| */ | ||
| public static function reset(): void { | ||
| self::$hook_prefix = ''; | ||
| self::$version = ''; | ||
| self::$container = null; | ||
| } |
There was a problem hiding this comment.
Why do we need test-only code in the production object? We could use reflection to forcibly clear the state in tests. Exposing this means it is something we will support and I don't know if we should.
There was a problem hiding this comment.
You are right
| /** | ||
| * @return array<string,array{0:string}> | ||
| */ | ||
| public function invalid_hook_prefixes(): array { | ||
| return [ | ||
| 'slash' => [ 'give/recurring' ], | ||
| 'space' => [ 'give recurring' ], | ||
| 'dot' => [ 'give.recurring' ], | ||
| 'backslash' => [ 'give\\recurring' ], | ||
| ]; | ||
| } |
There was a problem hiding this comment.
Could we instead use a Generator? This Array pattern that Claude likes to use unless otherwise specified I think is much harder to read personally.
Also, probably worth making it public static.
| public function test_it_accepts_letters_numbers_hyphens_and_underscores(): void { | ||
| Config::set_hook_prefix( 'give-recurring_2' ); | ||
|
|
||
| $this->assertSame( 'give-recurring_2', Config::get_hook_prefix() ); | ||
| } |
There was a problem hiding this comment.
Valid/invalid formats could just be a Data Provider, right? Could have the input prefix as one param and a result for "valid" and if it is false then we would expect an exception. Alternatively, we could have separate Data Providers and test methods for valid vs invalid, but overall I think it would be worthwhile to utilize Data Providers for this type of test.
| } | ||
|
|
||
| public function test_config_exception_is_catchable_as_a_runtime_exception(): void { | ||
| $this->expectException( RuntimeException::class ); |
There was a problem hiding this comment.
Why are we expecting RuntimeException here? Shouldn't we expect Config_Exception? This currently passes just because Config_Exception extends RuntimeException.
…g_State for test state management. Update tests to reflect changes and ensure proper exception handling for invalid hook prefixes.
2 participants
Configstatic facade — hook prefix, version, optional container — plusConfig_Exception.set_hook_prefix()rejects bad values, empty string included. Validating at read time would turn a bad prefix into a filter that silently never fires, surfacing later as a misleading "you must callset_hook_prefix()".Config_Exception extends RuntimeException, so callers get one catchable type across the library.Tests\Support\Test_Container, becauselucatume\DI52\Containerimplements PSR-11'sContainerInterface, not StellarWP's.set_container()'s signature is unchanged andstellarwp/container-contractstays the only production dependency.get_version()is stored but nothing reads it yet — spec known-issue F.