Open
Bug 694517
Opened 13 years ago
Updated 2 years ago
Use fatal asserts with dynamic suppression file generated from tbpl to aid assertion migration
Categories
(Firefox Build System :: General, enhancement)
Firefox Build System
General
Tracking
(Not tracked)
NEW
People
(Reporter: decoder, Unassigned)
References
(Blocks 1 open bug)
Details
Yesterday, Jesse and I had a discussion about asserts being fatal or not in our builds and I'm strongly on the side of making asserts fatal. I also did read through some bug reports and articles and from what I've learned to far, the problem seems to be that some older asserts are very noisy/being rather used as warnings while other/newer asserts could be fatal without causing too much trouble.
To aid the process of migrating towards fatal asserts, we thought it could be helpful to make all asserts fatal by default but have a dynamic decision about the fatality in the browser when the assert happens (per assert). This would allow to suppress those asserts that would be show stoppers right now while all the other asserts could be fatal.
The list of asserts that should be non-fatal by default could even be generated from Tinderbox. All known failing asserts showing up there don't need to be fatal in the debug browser build. A suppression file (e.g. similar to valgrind) could be served for that purpose.
Comment 1•13 years ago
|
||
As far as strategies for migrating towards fatal assertions: my existing plan, already implemented in reftest, and hopefully one of these days implemented for mochitest as well, is to annotate tests with the number (or numeric range) of assertions and report a test failure if the number differs. I think this has been working pretty well for the areas of the code covered by reftest -- we just need to do the same for mochitests (my patch to do that, in bug 404077 -- previously had problems with XPConnect assertions, though those may be resolved now).
Updated•7 years ago
|
Product: Core → Firefox Build System
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•