Closed Bug 454055 Opened 15 years ago Closed 13 years ago

Allow messages to be shown and highlighted in brief log

Categories

(Webtools Graveyard :: Tinderbox, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: sgautherie, Assigned: aki)

References

Details

Attachments

(1 file, 1 obsolete file)

This would easy noticing/fixing broken machines... (at least)

Example:

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1220791048.1220791111.3649.gz
WINNT 5.2 mozilla-central leak test build on 2008/09/07 05:37:28
[
nsinstall: cannot copy .\nspr4.dll to e:\builds\moz2_slave\mozilla-central-win32-debug\build\obj-firefox\dist\bin\nspr4.dll: The process cannot access the file because it is being used by another process.
make[5]: *** [export] Error 3
]

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1220791048.1220791111.3649.gz&fulltext=1
[
======== BuildStep started ========
'echo Building on: moz2-win32-slave05'
]
it could, but unless there's some reason that the build clients can't use the existing tinderbox parser to get the info you need displayed, i'd rather we not change the parser.
Assignee: nobody → build
Component: Tinderbox → Tinderbox Configuration
Product: Webtools → mozilla.org
QA Contact: tinderbox → ccooper
Version: Trunk → other
Is there a way to tell Tinderbox "hey! put this in the brief log!"? The reason we get this on Firefox3.0 is because the hostname is part of the column name. Obviously, we need a different solution here.
Assignee: build → aki
The brief log only contains errors currently.

Solution 1: We can make the hostname line look like an error using the existing error parsing.   For example, we could make it look like a cvs conflict, e.g. "C HOSTNAME=foo.mozilla.org".  The drawback here is that it will appear to be an error in the full log as well, and it's certainly possible that people will get confused when trying to actually find a cvs conflict in the log.

Solution 2: this is a 3-line showlog.cgi patch that will put any line beginning with "BriefMessage:" in the Build Error Summary in the brief log view only, not linked to any line in the Build Error Log.  The line will show up in the full log as-is, but only in the body of the message.

(I can change the behavior if that's desired).

I've set up tinderbox1 on my laptop and have verified it works here.  If we put this patch in place on the tinderbox server, then we need to alter the hostname line to start with "BriefMessage:" (sans quotes) to show in the brief tinderbox view.
Attachment #346764 - Flags: review?
Attachment #346764 - Flags: review? → review?(reed)
Component: Tinderbox Configuration → Tinderbox
Product: mozilla.org → Webtools
QA Contact: ccooper → tinderbox
Comment on attachment 346764 [details] [diff] [review]
add BriefMessage: functionality to showlog.cgi

Let's see if cls has any thoughts on if this is the best approach. I'll grab this back if cls can't get to it soon.
Attachment #346764 - Flags: review?(reed) → review?(cls)
We could potentially color code as well; e.g. the BriefMessage: lines are inside a <font color="green" style="font-color: #115511"></font> or something.
Comment on attachment 346764 [details] [diff] [review]
add BriefMessage: functionality to showlog.cgi

I'm lukewarm to the entire idea.  I don't think the Brief Log page needs to be cluttered with unnecessary info.  I'm going to imagine that if you're going to debug something, that you'll need to look at the full log anyway to get a clue about what's going on. 

In any case, I'd prefer if you would use TinderboxBriefMessage: or TinderboxLogMessage as the token instead of BriefMessage: .  

If you're going to start special casing the BriefMessages, then I'd argue that you should treat it just like another log error, which would mean putting the check down in output_summary_line().
Attachment #346764 - Flags: review?(cls) → review-
(In reply to comment #7)
> I don't think the Brief Log page needs to be cluttered with unnecessary info.

I disagree: the full log is there to "debug" things, yes; but the brief log should be enough to check that the failure is already known...
Renamed to TinderboxSummaryMessage:, moved to output_summary_line(), colored green, and placed in both the Brief and Full summary sections.

This is for information that may otherwise be buried in the full log, but would be helpful to know either while looking at the brief log, or looking at the full log before downloading the entire thing (or scrolling down and looking for it).

Previously we would have embedded this information in the tinderbox client name, but that also has the side effect of creating a new column.  Since our tinderbox pages are already overly large, having alternate methods of showing this information without having to parse through the entire full log would be helpful.
Attachment #346764 - Attachment is obsolete: true
Attachment #347364 - Flags: review?
Attachment #347364 - Flags: review? → review?(cls)
cls: any update on this patch review?
Assignee: aki → nobody
Attachment #347364 - Flags: review?(cls) → review+
Committed on tbox1_20080527_cls_branch

Checking in showlog.cgi;
/cvsroot/mozilla/webtools/tinderbox/showlog.cgi,v  <--  showlog.cgi
new revision: 1.29.2.4; previous revision: 1.29.2.3
Summary: Could 'Build Log (Brief)' include the true (slave) machine name (in case of error) ? → Allow messages to be shown and highlighted in brief log
(In reply to comment #11)
> Committed on tbox1_20080527_cls_branch
> 
> Checking in showlog.cgi;
> /cvsroot/mozilla/webtools/tinderbox/showlog.cgi,v  <--  showlog.cgi
> new revision: 1.29.2.4; previous revision: 1.29.2.3

Cool, thanks cls. 

hmm... dont know this area very well... now what? Do we have to do something to get this used in production? Or are we all done, and can close this bug?
(In reply to comment #12)
> hmm... dont know this area very well... now what? Do we have to do something to
> get this used in production? Or are we all done, and can close this bug?

The patch has yet to be committed to trunk, afaik. cls only committed it to his private branch.
Assignee: nobody → aki
Status: NEW → ASSIGNED
Keywords: checkin-needed
(In reply to comment #13)
> The patch has yet to be committed to trunk, afaik. cls only committed it to his
> private branch.

Where "private" has been redefined to mean "stable & unaffected by the crash landing of bug 398050 ".
mozilla/webtools/tinderbox/showlog.cgi 	1.33
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Works on tinderbox-stage.
http://tinderbox-stage.mozilla.org/showlog.cgi?log=MozillaTest/1257275043.1257275506.25217.gz

To fix this bug fully, we just have to prepend a "TinderboxSummaryMessage: " to the output from our hostname step.
Status: RESOLVED → VERIFIED
Product: Webtools → Webtools Graveyard
You need to log in before you can comment on or make changes to this bug.