Open intco opened 5 years ago
Hi @intco . Pay attention because $this->config
is different from $config
.
I know :-) I created specifically a new instance to demonstrate that default options from getDefaults will be overridden entirely. The application.name is lost even if i set only the application.secret.
Please, take a look to the source code of getDefaults to better understand it. It should be overridden to work as you want. Tell me in details if I'm missing something and if I'm missing your point.
consider the SimpleConfig class used in the test
https://github.com/hassankhan/config/blob/develop/tests/Fixture/SimpleConfig.php
its getDefaults() is:
If you instantiate the class providing an array like this:
$config = new SimpleConfig(
[
'application' => ['secret' => 'verysecret']
]
);
then this will fail
$this->assertEquals('configuration', $config->get('application.name'));
because $this->data
is now
[
'host' => 'localhost',
'port' => 80,
'servers' => [
'host1',
'host2',
'host3'
],
'application' => [
'secret' => 's3cr3t'
]
];
just add these lines to your local copy of AbstractConfigTest and try yourself
Thanks for the detailed explanation @intco , now it's clear. 😄 It's an issue and you are right about it.
A possible solution could be using array_merge_recursive
instead of array_merge
here:
What do you think about it?
I've tried with array_merge_recursive but a lot of tests failed, see below
> .\vendor\bin\phpunit
PHPUnit 7.5.1 by Sebastian Bergmann and contributors.
Runtime: PHP 7.1.17 with Xdebug 2.6.1
Configuration: (project-dir)phpunit.xml.dist
FF.F...F.........FFFF.........F..F............................... 65 / 72 ( 90%)
....... 72 / 72 (100%)
Time: 2.35 seconds, Memory: 8.00MB
There were 10 failures:
1) Noodlehaus\AbstractConfigTest::testDefaultOptionsSetOnInstantiation
Array (...) does not match expected type "string".
(project-dir)tests\AbstractConfigTest.php:62
2) Noodlehaus\AbstractConfigTest::testGet
Array (...) does not match expected type "string".
(project-dir)tests\AbstractConfigTest.php:71
3) Noodlehaus\AbstractConfigTest::testGetNestedKey
Array (...) does not match expected type "string".
(project-dir)tests\AbstractConfigTest.php:87
4) Noodlehaus\AbstractConfigTest::testGetReturnsArray
Array (...) does not match expected type "string".
(project-dir)tests\AbstractConfigTest.php:120
5) Noodlehaus\AbstractConfigTest::testAll
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
Array (
- 'host' => 'localhost'
- 'port' => 80
+ 'host' => Array (...)
+ 'port' => Array (...)
'servers' => Array (
0 => 'host1'
1 => 'host2'
2 => 'host3'
+ 3 => 'host1'
+ 4 => 'host2'
+ 5 => 'host3'
)
'application' => Array (
- 'name' => 'configuration'
- 'secret' => 's3cr3t'
+ 'name' => Array (...)
+ 'secret' => Array (...)
'runtime' => null
)
'user' => null
)
(project-dir)tests\AbstractConfigTest.php:288
6) Noodlehaus\AbstractConfigTest::testMerge
Array (...) does not match expected type "string".
(project-dir)tests\AbstractConfigTest.php:304
7) Noodlehaus\AbstractConfigTest::testOffsetGet
Array (...) does not match expected type "string".
(project-dir)tests\AbstractConfigTest.php:312
8) Noodlehaus\AbstractConfigTest::testOffsetGetNestedKey
Array (...) does not match expected type "string".
(project-dir)tests\AbstractConfigTest.php:320
9) Noodlehaus\AbstractConfigTest::testIterator
Array (...) does not match expected type "string".
(project-dir)tests\AbstractConfigTest.php:485
10) Noodlehaus\AbstractConfigTest::testDefaultOptionsSetOnInstantiationDoNotOverwriteSections
Array (...) does not match expected type "string".
(project-dir)tests\AbstractConfigTest.php:514
FAILURES!
Tests: 72, Assertions: 142, Failures: 10.
Hi @intco, thanks for the detailed response and for posting your findings. I haven't had the chance to properly look, but I think those failing test cases are mainly due to test fixtures that weren't set up for recursive merging of properties.
currently you can not overwrite a single key (e.g. 'secret' in the test method) without overwriting the whole section (e.g. 'application' in the test method).