Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Config.ini.php shouldn't affect test enviornament. #9484

Open
sebastianpiskorski opened this issue Jan 8, 2016 · 1 comment
Open

Config.ini.php shouldn't affect test enviornament. #9484

sebastianpiskorski opened this issue Jan 8, 2016 · 1 comment
Labels
c: Tests & QA For issues related to automated tests or making it easier to QA & test issues.

Comments

@sebastianpiskorski
Copy link
Contributor

If I set some value in config.ini.php

[MySection]
key = value

running any test I get:

$config = Config::getInstance();
var_dump($config->MySection);
//Output:
array(1) {
  'key' =>
  string(5) "value"
}

it shouldn't be visible from tests, shouldn't be set implicitly in environment. Tests should use only global.ini.php, and explicit TestConfig class.

Why is this bad?

  • One config might affect many places and many tests, thus test can give fake results.
  • Every test should be isolated as much as possible, especially from local instance settings.
  • Every test should set its own Config, so it is possible to test many various cases. Config values might exclude some scenarios. Eg. If you have set config for scenario A it might affect scenario B and give fake result.
  • There might be a test that will have to physically write to config. If value will stay in config it might break some other tests.
@mattab
Copy link
Member

mattab commented Jan 27, 2016

👍 absolutely agree. Our tests shouldn't depend on the local configuration file. a pull request would be very welcome, or we will investigate in coming months / during 3.0.0 dev cycle

@mattab mattab added the Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical. label Jan 27, 2016
@mattab mattab added this to the 3.0.0 milestone Jan 27, 2016
@mattab mattab modified the milestones: Mid term, 3.0.0 Dec 1, 2016
@mattab mattab added c: Tests & QA For issues related to automated tests or making it easier to QA & test issues. and removed Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical. labels Dec 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: Tests & QA For issues related to automated tests or making it easier to QA & test issues.
Projects
None yet
Development

No branches or pull requests

2 participants