Re: [RFC] We deserve better than runtime warnings
Well, for one thing, it's not self-documenting. For the other, unless I'm missing something, we won't notice if an assertion is fixed and replaced with another one. And yes, catching when an assertion is fixed would clearly be useful, too. Cheers, David On 20/11/14 18:14, L. David Baron wrote: It's not all that fragile, since most tests don't have known assertions, so in that vast majority of tests, we have test coverage of regressions of assertion counts. This covers reftests, crashtests, and all mochitests except for mochitest-browser-chrome (bug 847275 covers doing it there). (We should probably make an attempt to go through the tests that have assertion count ranges including 0, since nothing catches when those assertions are fixed, and we're missing the test coverage there.) -David -- David Rajchenbach-Teller, PhD Performance Team, Mozilla signature.asc Description: OpenPGP digital signature ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: [RFC] We deserve better than runtime warnings
On 11/21/14 8:49 AM, L. David Baron wrote: On Friday 2014-11-21 12:51 +0100, David Rajchenbach-Teller wrote: Well, for one thing, it's not self-documenting. We should comment them better (i.e., have a bug on each one, and point to the bug in a comment on the expectAssertions line). I wasn't able to do that when initially landing the assertion checking because, at the time, there were too many to keep up with the tree. At this point I could probably go back through the data I used for that to annotate the remaining ones. A self-documenting expectAssertions API might take an array of bug numbers (for expected assertions) as a function argument instead of an expected assertion count. For extra credit, expectAssertions could use the Bugzilla API to validate the bug numbers are relevant assertions and not RESOLVED FIXED. :) chris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: [RFC] We deserve better than runtime warnings
I wasn't aware that we could whitelist an individual NS_ASSERTION. How do we do that? On 20/11/14 17:24, Boris Zbarsky wrote: This sounds lovely. Note that in C++ for some of our test suites we already have this with NS_ASSERTION and for all test suites we have it with MOZ_ASSERT. Assuming, of course, that the test suite is run in debug builds... -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform -- David Rajchenbach-Teller, PhD Performance Team, Mozilla signature.asc Description: OpenPGP digital signature ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: [RFC] We deserve better than runtime warnings
I believe that we can provide something less fragile than that. On 20/11/14 17:56, Boris Zbarsky wrote: Ah, we can't. We can whitelist the number of assertions in a mochitest (or a number range if the number is not quite stable), but not the text of the assertion. -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform -- David Rajchenbach-Teller, PhD Performance Team, Mozilla signature.asc Description: OpenPGP digital signature ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: [RFC] We deserve better than runtime warnings
On 20/11/14 17:56, Boris Zbarsky wrote: Ah, we can't. We can whitelist the number of assertions in a mochitest (or a number range if the number is not quite stable), but not the text of the assertion. On Thursday 2014-11-20 18:05 +0100, David Rajchenbach-Teller wrote: I believe that we can provide something less fragile than that. It's not all that fragile, since most tests don't have known assertions, so in that vast majority of tests, we have test coverage of regressions of assertion counts. This covers reftests, crashtests, and all mochitests except for mochitest-browser-chrome (bug 847275 covers doing it there). (We should probably make an attempt to go through the tests that have assertion count ranges including 0, since nothing catches when those assertions are fixed, and we're missing the test coverage there.) -David -- 턞 L. David Baron http://dbaron.org/ 턂 턢 Mozilla https://www.mozilla.org/ 턂 Before I built a wall I'd ask to know What I was walling in or walling out, And to whom I was like to give offense. - Robert Frost, Mending Wall (1914) signature.asc Description: Digital signature ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: [RFC] We deserve better than runtime warnings
L. David Baron writes: On 20/11/14 17:56, Boris Zbarsky wrote: Ah, we can't. We can whitelist the number of assertions in a mochitest (or a number range if the number is not quite stable), but not the text of the assertion. On Thursday 2014-11-20 18:05 +0100, David Rajchenbach-Teller wrote: I believe that we can provide something less fragile than that. It's not all that fragile, since most tests don't have known assertions, so in that vast majority of tests, we have test coverage of regressions of assertion counts. This covers reftests, crashtests, and all mochitests except for mochitest-browser-chrome (bug 847275 covers doing it there). runcppunittests.py, runxpcshelltests.py, and rungtests.py use XPCOM_DEBUG_BREAK=stack-and-abort so we should have coverage there too. When web-platform-tests run with debug builds, we can do the same there because known crashes can be annotated in those tests. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: [RFC] We deserve better than runtime warnings
There is a priority list of best to worst something like this: 1. Types 2. Compile time assertions 3. Unit tests 4. Fatal run time assertions 5. Non-fatal runtime assertions 6. Documentation This is the order in which you are most likely to quickly find a problem. Obviously 1 and 2 don't apply to Javascript although static analysis could be the equivalent. Anthony On 21/11/14 05:51, David Rajchenbach-Teller wrote: I wasn't aware that we could whitelist an individual NS_ASSERTION. How do we do that? On 20/11/14 17:24, Boris Zbarsky wrote: This sounds lovely. Note that in C++ for some of our test suites we already have this with NS_ASSERTION and for all test suites we have it with MOZ_ASSERT. Assuming, of course, that the test suite is run in debug builds... -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
