We missed converting one the variable_get('error_level') in the original conversion. This patch fixes this bug.

Part of #1775842: [meta] Convert all variables to state and/or config systems

Files: 
CommentFileSizeAuthor
#9 1798756-9.error_level.patch2.98 KBalexpott
PASSED: [[SimpleTest]]: [MySQL] 41,908 pass(es).
[ View ]
#9 1798756-9.error_level.test-only.patch2.34 KBalexpott
FAILED: [[SimpleTest]]: [MySQL] 41,916 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#6 1798756-error_level_2.patch657 bytesandreiashu
PASSED: [[SimpleTest]]: [MySQL] 41,893 pass(es).
[ View ]
#2 1798756-error_level_1.patch688 bytesandreiashu
PASSED: [[SimpleTest]]: [MySQL] 41,899 pass(es).
[ View ]

Comments

Title:Convert error_level to CMIFinish conversion of error_level to CMI
Priority:Normal» Critical

Spoke with @alexpott and it seems this has already been addressed in a separate issue but we discovered one place where it was missed. Patch to come

Status:Active» Needs review
StatusFileSize
new688 bytes
PASSED: [[SimpleTest]]: [MySQL] 41,899 pass(es).
[ View ]

patch attached

+++ b/core/includes/errors.incundefined
@@ -250,7 +250,7 @@ function _drupal_log_error($error, $fatal = FALSE) {
+      $error_level = config('system.logging')->get('error_level') ?: ERROR_REPORTING_DISPLAY_ALL;

@alexpott: does it make sense to have to default to a value at code level? We already have a default in system.logging.yml

Status:Needs review» Needs work

Issue tags:+Configuration system

tagging

Status:Needs work» Needs review
StatusFileSize
new657 bytes
PASSED: [[SimpleTest]]: [MySQL] 41,893 pass(es).
[ View ]

Removed the redundant default value

Category:task» bug
Status:Needs review» Reviewed & tested by the community

Looks great.

Thanks for the work!

Status:Reviewed & tested by the community» Needs review

Just a quick question. Is this actually causing a bug to appear somewhere? If so, we need tests to illustrate the failure.

StatusFileSize
new2.34 KB
FAILED: [[SimpleTest]]: [MySQL] 41,916 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
new2.98 KB
PASSED: [[SimpleTest]]: [MySQL] 41,908 pass(es).
[ View ]

Yep webchick you're right we need a test...

So here it is. The .test.only.patch should fail :)

Status:Needs review» Needs work

The last submitted patch, 1798756-9.error_level.test-only.patch, failed testing.

Status:Needs work» Needs review

Setting back to RTBC as the added test proves the bug and the fix!

Status:Needs review» Reviewed & tested by the community

Looks good

Small nit going forward.... please do not use t() around assert messages. Thanks.

Lars that t() is in all the ErrorHandlerTest's - copy/paste mistake :)

There are patches waiting review/commit removing all of the t() from assert messages. Hopefully, soon there won't be bad examples going forward.

Status:Reviewed & tested by the community» Fixed

Awesome, thanks a lot!

Committed and pushed to 8.x.

For reference, this is the issue cleaning up t() around assert messages in the referenced test file: #1797926: Remove t() from asserts from System sub-system tests.

@alexpott: re #7: thanks for the guidance and helping out :)

Yep, one t() added with commit

+++ b/core/modules/system/lib/Drupal/system/Tests/System/ErrorHandlerTest.phpundefined
@@ -53,6 +53,15 @@ class ErrorHandlerTest extends WebTestBase {
+    $this->assertResponse(200, t('Received expected HTTP status code.'));

this place should be fixed in #1797926: Remove t() from asserts from System sub-system tests

Automatically closed -- issue fixed for 2 weeks with no activity.

Issue summary:View changes

Updated issue summary.