On 11/23/2012 10:45 AM, Eric Rescorla wrote:
On Fri, Nov 23, 2012 at 1:34 AM, Henrik Skupin <[email protected]> wrote:
3. We establish a review policy that any patches that can definitely
have a crashtest associated with them has to be included on check-in
by the developer building the patch. If the patch comes in without a
crashtest that can indeed possibly have one, the patch gets a review-
until the test is included. That will balance both validating that
the patch actually does work at check-in, but also starts to burn
through progressively building out a crashtest regression automation
suite.
I would love that because of:
* it helps devs to check that the crash has been fixed across platforms
by pushing it to try (via the alder branch for now) instead of only
testing it on their own platform work happens.
* crashtests are easy to create from the already attached testcases.
I agree with Randell that while this may be good practice in many
cases, it's not a reasonable general policy as a gating factor on
checkin. As I said, to the extent to which test cases are already
available (or Test develops them) I agree that they should pass,
but I don't think it's a requirement that the developers write them
for every crash.
Most of us can test cross-platform if we want or feel a need to. And a
fair number of the crashes are ASAN failures, which we can't easily
test. As very few of our bugs are platform dependent (a few in gum are,
for example), cross-platform testing doesn't add much to our tests.
More than nothing, but not a ton at this stage in development.
I agree with Jason (and Henrik?) that developers for WebRTC should try
to indicate using in-testsuite?/+/- what they think the appropriate use
of tests is on checkin. (I'm guessing that the admonition about "only
QA actively working on testcases in this component should use this
keyword" is not actually the case?) But even if a bug should have a
test, if we don't have one yet, I'd mark it in-testsuite?, and comment
on the bug about what type of test (I'd generally prefer mochitests!)
should be done and how broad it should be (right now, I like broad if it
also hits the bug).
I'll be clear: we will not at this point block r+'s for bugs that
can/should have a test.
If a crash fix should have a test, I'd love to get a test with the
patch, but at this point it's more important to fix the bug - please
mark it "in-testsuite?" and we'll try to get a test in place ASAP. Also,
please look at the existing tests and see if one can be easily/safely
modified to include the new bug.
--
Randell Jesup, Mozilla Corp
_______________________________________________
dev-media mailing list
[email protected]
https://lists.mozilla.org/listinfo/dev-media
