Tests would fail on windows because of /tmp usage, but it turns out it's not even unnecessary.
Tests would save SampleConfigFile class in /tmp/SampleConfigFile.php, because there once was CustomConfig that could load files, so to test that /tmp/SampleConfigFile.php was created for testing. Later, it was removed in favour of SampleConfig, but instead of removing /tmp/SampleConfigFile.php, it was left as it was, and then immediately include_once. Obviously this is unnecessary since the file isn't tested, only the class that's passed to registerConfig(). Without the file, the test doesn't need to operate on filesystem, so tests pass on windows.
Tests were reliant on the order and the fact that multiple tests are run. If you run any test that uses registerConfig() but not reload() (so shouldReadMultipleSampleConfigs(), shouldReadSampleConfig(), shouldReturnConfigValue()), they would fail if you run them separately.
Further more, because the config remembers the state between tests (static field), the tests are sometimes false-positive, so even if you remove the code from given/when it would pass, because other tests registered the config. They would be caught, if the tested values were different, but SampleConfig always returned the same value for each test, so each assertion passed.
To fix it, I made new SampleConfig() into new SampleConfig(['foo' => 'bar']), so each test recieves unique key=>value pair, and isn't prone to false positives from other configs.
Feel free to squash the commits while merging the PR.
Tests would fail on windows because of
/tmp
usage, but it turns out it's not even unnecessary.Tests would save
SampleConfigFile
class in/tmp/SampleConfigFile.php
, because there once wasCustomConfig
that could load files, so to test that/tmp/SampleConfigFile.php
was created for testing. Later, it was removed in favour ofSampleConfig
, but instead of removing/tmp/SampleConfigFile.php
, it was left as it was, and then immediatelyinclude_once
. Obviously this is unnecessary since the file isn't tested, only the class that's passed toregisterConfig()
. Without the file, the test doesn't need to operate on filesystem, so tests pass on windows.Tests were reliant on the order and the fact that multiple tests are run. If you run any test that uses
registerConfig()
but notreload()
(soshouldReadMultipleSampleConfigs()
,shouldReadSampleConfig()
,shouldReturnConfigValue()
), they would fail if you run them separately.Further more, because the config remembers the state between tests (
static
field), the tests are sometimes false-positive, so even if you remove the code from given/when it would pass, because other tests registered the config. They would be caught, if the tested values were different, butSampleConfig
always returned the same value for each test, so each assertion passed.To fix it, I made
new SampleConfig()
intonew SampleConfig(['foo' => 'bar'])
, so each test recieves uniquekey=>value
pair, and isn't prone to false positives from other configs.Feel free to squash the commits while merging the PR.