https://bugs.winehq.org/show_bug.cgi?id=48911
Bug ID: 48911 Summary: Check for compilation warnings Product: Wine-Testbot Version: unspecified Hardware: x86 OS: Linux Status: NEW Severity: normal Priority: P2 Component: unknown Assignee: wine-bugs@winehq.org Reporter: fgouget@codeweavers.com Distribution: ---
Compiling Wine with -Werror is troublesome because it applies blindly to both wine-devel patches and patches submitted manually via the website. The sole purpose of the latter is often to just clarify some point for the development of another patch and are not meant to be submitted. Thus failing on warnings just makes running these quick tests more cumbersome.
So the best way forward would be for the TestBot to identify and report warnings but not fail the compilation and abort the tests when they occur.
So: 1. The builds should happen without -Werror.
2. GetLogLineCategory() should identify the warnings and add them to LogInfo but distinguish them from errors (replace IsNew with a Category field?).
3. Compare task.log to the reference build log to distinguish new warnings from old ones. This is necessary in case the compiler used by the TestBot produces warnings on the unpatched Wine tree. It will require storing the build logs in latest/ just like the TestBot already does for the reference WineTest reports.
4. LogInfo's will need to keep track of the number of new warnings which really means renaming the 'NewCount' field to avoid confusion.
5. The warnings should be saved in the errors files. The new line types could be 'w' and 'W' for old and new warnings respectively (then it would make sense to use e and E for errors).
6. This will let the WineRun*() scripts know about both warnings and errors. They can then decide what to do depending on the type of job and whether the warnings are new or not. - Old warnings would always be ignored. - If a build task has new warnings, instead of marking it as 'badbuild' it could be marked as 'completed' as usual but with a non-zero number of failures. This would probably allow running the tests on Windows but the patch would still be considered bad since the job would end up with a non-zero number of failures. - For Wine tasks the number of warnings would be added to the Failures field for the same overall result. - So in both cases the non-zero number of failures would only impact wine-devel patches.
7. The JobDetails page should show the warnings in summary mode and highlight them in the full build log. That would make it easy for developers to see the warnings on web-submitted jobs, and thus fix them before submitting the patch to wine-devel.
https://bugs.winehq.org/show_bug.cgi?id=48911
François Gouget fgouget@codeweavers.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |RESOLVED Resolution|--- |FIXED
--- Comment #1 from François Gouget fgouget@codeweavers.com --- The TestBot now supports multiple messages types in the .errors files: * e/E : The usual old / new errors / failures. * k/K : The known old / new failures. * w/W : The old / new warnings.
The job details page shows all messages types which includes the warnings and the task object has counters for the old/new warnings too.
The core functionality was added in these commits:
commit 28c6e0ade18f90b5d42b9ee15edf62b939167684 Author: Francois Gouget fgouget@codeweavers.com Date: Wed Oct 19 17:41:11 2022 +0200
testbot: Add a message category for warnings.
This adds support for identifying warnings (e.g. build warnings), and highlighting or reporting them.
commit f1fe0ca977f17a1cb5a631e4d4c1c2f0c77a244f Author: Francois Gouget fgouget@codeweavers.com Date: Wed Oct 19 19:19:07 2022 +0200
testbot: Track which build warnings are new.
This requires keeping the past build logs as a reference and comparing the new build logs to them. Note that because of the build parallelism the warnings may not always appear in the same order in the reference build logs so there could still be some false positives. If that happens the false positives can be avoided by tracking the warnings as known failures. Regardless, even new warnings do not cause a patch to be reported as bad.
commit cc2b971a9c02838154cf94171e539a846133faf7 Author: Francois Gouget fgouget@codeweavers.com Date: Fri Oct 28 02:56:25 2022 +0200
testbot: Add Task fields to track warnings, failures and bad test units.
So far the tasks have a TestFailures field which tracks the number of failures for that task, but not whether any of them is new. This adds a field to keep track of the number of new failures and another pair to do the same for warnings. This further adds a field to track the numbder of failed test units. These fields can then be used in the web pages to indicate whether there are new failures / warnings, and how many test units have failed. But until there is support for setting these fields, they are hidden.
commit d7f66da4f6092ecb26ff40a3111a9208406f1f73 Author: Francois Gouget fgouget@codeweavers.com Date: Thu Nov 3 03:10:54 2022 +0100
testbot/LogUtils: Track the known new failures and warnings.
This adds a per-message-type KnownNew counter to the $LogInfo structure which allows callers to quickly discount the new but known failures and warnings.
commit de42885fb59d7d5f86b8dbf857fb6622eaa16c25 Author: Francois Gouget fgouget@codeweavers.com Date: Fri Oct 28 03:00:51 2022 +0200
testbot/WineRun*: Set the new task failure and warning counters.
commit 4977ed2e9d702fc57e0f94fd9e25dc18a9f9c3b5 Author: Francois Gouget fgouget@codeweavers.com Date: Fri Oct 28 03:01:16 2022 +0200
testbot/web: Show the number of new failures and warnings.
This makes it easier to see which jobs and tasks have new failures. This also makes it obvious when a patch adds new compilation warnings.
commit 5683ae9206971084b0b6827f84d6c36ee196496d Author: Francois Gouget fgouget@codeweavers.com Date: Thu Dec 8 18:44:16 2022 +0100
testbot/LogUtils: Fix counting of known new messages.
A given line can be matched by more than one known failure when it is too generic. This resulted in the eKnownNew counter being too high which then resulted in an invalid NewTestFailures value for the task. This patch ensures the *KnownNew counters count each line only once.
https://bugs.winehq.org/show_bug.cgi?id=48911
François Gouget fgouget@codeweavers.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED
--- Comment #2 from François Gouget fgouget@codeweavers.com --- In production, closing.