Make DB connection name configurable #14 #23
Conversation
``connection_name`` is not common among all sensors.
…getSetting() This will prevent the creation of redundant boiler plate code in sensors.
| * @return string The name of the connection to check. | ||
| * @todo Cover by a test. | ||
| */ | ||
| protected function _getSetting(string $name, $default = null): string |
There was a problem hiding this comment.
@ravage84 I'm not sure how to test this.
Maybe it should be public? Or maybe you have an idea on how to test it.
There was a problem hiding this comment.
No, it doesn't need to. Sure it would make testing easier, but code doesn't need to change just for the sake of testability.
This can be tested by a test dummy. A class that extends Sensor and then implements a public method which simply calls _getSettings() and returns it.
This public method can be called in the test case and its return value be assserted. The to-be-asserted value can be put into the configuration, beforehand, naturally.
There was a problem hiding this comment.
On a second thought, we might want to move this method to the Config class and make it public. This way the code is where it should be AND is easier testable... Let's discuss this tomorow.
| @@ -60,6 +73,7 @@ public function testDefaultConfig() | |||
| $this->assertEquals(true, $sensorConfig->getEnabled()); | |||
| $this->assertEquals(2, $sensorConfig->getSeverity()); | |||
| $this->assertEquals(null, $sensorConfig->getClass()); | |||
| $this->assertEquals([], $sensorConfig->getSettings()); | |||
There was a problem hiding this comment.
This test is fine, but I think we should test the individual methods in their own test cases, too.
I'll do that once we merged this.
@ravage84 I tried implementing your suggestion in #14, please take a look 😇
Closes #14