Injection Inception, Change #4: Using DI as primary mechanism in setting up Test environment #8008
Conversation
diosmosis
added
c: Platform
diosmosis
force-pushed
the
test_env_di_5
branch
3 times, most recently
from
4f7b4b8
to
da404e3
Compare
diosmosis
added
the
c: Tests & QA
diosmosis
added
Needs Review
|
Ready for review and (possibly) merge. |
| 'ini.log.log_level' => $level, | ||
| 'ini.log.string_message_format' => self::STRING_MESSAGE_FORMAT, | ||
| 'ini.log.logger_file_path' => self::getLogFileLocation(), | ||
| 'Psr\Log\LoggerInterface' => \DI\get('Monolog\Logger') |
There was a problem hiding this comment.
Could these 3 lines be moved to provideContainerConfig() or does the new environment completely ignores the current one? If that's the case, then why do we bind LoggerInterface in provideContainerConfig()? (isn't that useless?)
There was a problem hiding this comment.
provideContainerConfig() is called when the test case creates a new Environment instance, so we'd have to call it here if we put the other lines in that method. But you're right, since we create a new Environment in order to inject the Logger into Log, we don't need the provideContainerConfig(), just forgot to remove it.
Note: Will remove in a new commit since PRs depend on this one.
|
Overall it looks good, moving some test config to For the file Except that 👍 System tests are failing but it seems to be a random failure since AllTests is green. |
…ass to Piwik\Tests\Framework namespace. Remove manual require statements.
…since that's where it is used.
…y entry points will apply overridden DI config.
…logger), we must enable it in a test class override method, since this test checks the log output. Fixing LogTest now that NullLogger is used for test environment setup. Make sure translations are loaded in EnvironmentValidationTest.php. Make sure the logger is enabled for ArchiveCronTest.
…me reason cron archiver will fail when archiving segments if translations aren't available. Segment archiving requests return the token General_DateRangeFromTo w/o anything else (only some of them).
Fix Fixture class reference in MultiSites' ControllerTest. Fix Fixture class reference in ExamplePlugin system test.
…sing singleton instance.
…default to using mock access.
… not to my knowledge been useful in debugging mysterious test failures.
…ment.bootstrapped one.
…ironment.bootstrapped one.
…o calls to StaticContainer::addDefinitions will stack. This allows us to override global.observers in tests.
…ing authentication hack (which no longer works since test environment setup is unified), add an observer to Tracker.newHandler which throws an exception and returns the HTTP status code 500.
Injection Inception, Change #4: Using DI as primary mechanism in setting up Test environment
Includes various changes to how the test environment is setup, focusing on using DI as the primary overriding mechanism.
Changes include:
config/environment/test.php.config/environment/test.php) by merging some together.observers.globalin individual tests.Piwik\Tests\Frameworknamespace.'test'environment string into normal Piwik entry points AND means we can combine the test environment w/ other environments (like the tracker one).provideContainerConfigBeforeClassnot being recognized by TestingEnvironment.php. ThetestCaseClassTestingEnvironment property was not set in Fixture.php, so it didn't get propagated to other child processes.FakeAccess::clearAccess()so it could be explicitly cleared.Everything else are fixes for tests. Individual commit messages should explain what the changes were and why they were added.