Per the debug messages to be provided by #296574: Provide debug facility for general and testing it would seem logical to color them differently for easy distinguishing.

I think just using the group property of assertions should work. Look for group "debug".

CommentFileSizeAuthor
#14 500334-simpletest-debug-color.png42.44 KBboombatower
#2 500334-simpletest-debug-color.patch1.48 KBboombatower
#1 500334-simpletest-debug-color.patch1.48 KBboombatower

Comments

boombatower’s picture

boombatower commented
Assigned: Unassigned » boombatower
Status: Active » Needs review
StatusFileSize
new 500334-simpletest-debug-color.patch1.48 KB
boombatower’s picture

boombatower commented
StatusFileSize
new 500334-simpletest-debug-color.patch1.48 KB

Changed colors to whites.

Status: Needs review » Needs work

The last submitted patch failed testing.

boombatower’s picture

boombatower commented
Status: Needs work » Needs review

Test slave crapped.

boombatower’s picture

boombatower commented
Status: Needs review » Reviewed & tested by the community

I think this is fairly straight forward.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

I'm not sure why this is a separate patch? There's no point in committing it if #296574: Provide debug facility for general and testing doesn't get in, no? Can we move this code into the patch over there?

boombatower’s picture

boombatower commented

Actually if you use the group 'Debug' you can take advantage of this. Either way it seemed like a separate patch, but we can merge.

/me has been burned TOO many times over the whole large patch stuff.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Closed (won't fix)

Ok, boombatower clarified on IRC that this patch is still useful even right now if you do something like:

$this->assertTrue(FALSE, 'message', 'Debug');

However, I tried this and it sends conflicting messages. The results says "1 fail" yet none of the rows are coloured red. There's only the one grey "Debug" message row with a red X next to it. If I change it to a pass instead (assertTrue(TRUE...), then the fieldset gets collapsed and I need to explicitly expand it to read the debug message. Again, not intuitive.

Therefore, I'm not sure this makes much sense by itself. Let's roll it in with a patch that adds an explicit debug function.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Closed (won't fix) » Needs review

On IRC, Jimmy explained that this actually is a follow-up to the verbose mode patch which is already using this grouping for debug messages.

Just a question. Why use the grouping? Why not a different $assertion->status;?

boombatower’s picture

boombatower commented

I would much rather not add a debug status as that would require a rewrite of parts of the code, considering these messages are purely for debug that seems a bit much.

Verbose uses the Debug group right now and #296574: Provide debug facility for general and testing will provide a function to call for general debug info. So either commit here or I can include in the other debug patch.

boombatower’s picture

boombatower commented
Status: Needs review » Reviewed & tested by the community

Extremely simple followup

dries’s picture

dries commented
Status: Reviewed & tested by the community » Needs review

Screenshot?

boombatower’s picture

boombatower commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new 500334-simpletest-debug-color.png42.44 KB

Can do.

simpletest debug color

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Ok, committed to HEAD.

Status: Fixed » Closed (fixed)

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