Problem/Motivation
Bet you didn't know DrupalKernel has Simpletest in it.
DrupalKernel::bootEnvironment() says this:
// Indicate that code is operating in a test child site.
if (!defined('DRUPAL_TEST_IN_CHILD_SITE')) {
if ($test_prefix = drupal_valid_test_ua()) {
$test_db = new TestDatabase($test_prefix);
// Only code that interfaces directly with tests should rely on this
// constant; e.g., the error/exception handler conditionally adds further
// error information into HTTP response headers that are consumed by
// Simpletest's internal browser.
define('DRUPAL_TEST_IN_CHILD_SITE', TRUE);
// Web tests are to be conducted with runtime assertions active.
assert_options(ASSERT_ACTIVE, TRUE);
// Now synchronize PHP 5 and 7's handling of assertions as much as
// possible.
Handle::register();
// Log fatal errors to the test site directory.
ini_set('log_errors', 1);
ini_set('error_log', $app_root . '/' . $test_db->getTestSitePath() . '/error.log');
// Ensure that a rewritten settings.php is used if opcache is on.
ini_set('opcache.validate_timestamps', 'on');
ini_set('opcache.revalidate_freq', 0);
}
else {
// Ensure that no other code defines this.
define('DRUPAL_TEST_IN_CHILD_SITE', FALSE);
}
}
This code fragment exists so that we can test WebTestBase.
When we eventually deprecate TestBase/WebTestBase, we won't be able to deprecate DrupalKernel::bootEnvironment().
Proposed resolution
Factor this code out of bootEnvironment() and into a separate static method which can be deprecated.
Deprecate the new method.
Somehow formally deprecate usages of DRUPAL_TEST_IN_CHILD_SITE which exists solely to isolate TestBase and WebTestBase from themselves during testing.
Remaining tasks
Figure out how to deprecate drupal_valid_test_ua() and friends.
User interface changes
API changes
Data model changes
Comments
Comment #2
mile23Something like this.
Comment #4
mile23Reroll.
Some of the fails in #2 fail locally for 8.6.x, so I'm curious to see what happens here.
Comment #5
mile23Injecting the app root.
Comment #7
mile23I honestly though there would be fails in #5.
Method docblock cleanup and CS fix.
Added @deprecated to the docblock. We're not ready to trigger a deprecation error by any means, since this called on every
DrupalKernel::bootEnvironment()call. Added a @todo for that eventually. We'll need a follow-up if this is the way to go.Adding 'needs CR' tag, but holding off on authoring it until there's some consensus.
Updating IS with a little bit of scope shift, because we should just go ahead and mark this method as deprecated.
Comment #8
mile23Comment #9
lendude@Mile23++
Instead of trigger_error should this just be a private method and deprecated? That way nobody outside core can ever use it and adding a trigger_error to notify users outside of core is not needed. Doing a trigger error here just sounds weird, the moment we remove the call to the method from core, we also remove the only use of the method anywhere, so what would be the point of raising the error.
why is this needed? It seems like this is just reshuffling code. Am I missing something?
Comment #10
mile23Thanks.
This patch turns the new method into a private one.
Added a CR, with @see in the docblock.
The skipped deprecation slipped in by accident.
Comment #11
lendudeNice. Lets see what others think.
Comment #12
dawehnerI'm quite confused about this issue. This is needed for BrowserTestBase and Nightwatch based testing. It is the way how we separate requests from the main and child sites.
Comment #13
lendude@dawehner I saw some explicit checks for DRUPAL_TEST_IN_CHILD_SITE in Webtestbase et al. but not in BrowserTestBase or anything else. But looking at it in some more detail, you are right, it does seem to be a bit more wide spread in its use. So if we want to get rid of this, we should handle the existing dependencies too, I guess.
Comment #14
dawehnerI guess "needs work" is a bit more specific :)
Comment #16
MerryHamster commentedonly reroll #10 patch for 8.7.x
Comment #17
MerryHamster commentedComment #19
volegerWe have some progress in deprecation of the
drupal_valid_test_ua()related functions #3038513: Move drupal_generate_test_ua() into the test systemComment #26
alexpottI agree with #12. DRUPAL_TEST_IN_CHILD_SITE is used for testing not just Simpletest and the removal of Simpletest is not enough to remove it. This is the system working as designed.