David Rajchenbach Teller: RFC: We deserve better than runtime warnings |
Consider the following scenario:
How often has this happened to everyone of us?
This scenario has many variants (e.g. module A changed and nobody realized that module B is now in a situation it misuses module A), but they all boil down to the same thing: runtime warnings are designed to be lost, not fixed. To make things worse, many of our warnings are not actionable, simply because we have no way of knowing where they come from – I’m looking at you, Cu.reportError.
We would certainly save considerable amounts of time if warnings caused immediate assertion failures, or alternatively test failures (i.e. fail, but only when running the unit tests). Unfortunately, we can do neither, as we have a number of tests that trigger the warnings either
However, I believe that causing test failures is still the solution. We just need a mechanism that supports a form of whitelisting to cope with the aforementioned cases.
RuntimeAssert is an experiment at provoding a standard mechanism to replace warnings. I have a prototype implemented as part of bug 1080457. Feedback would be appreciated.
The key features are the following:
RuntimeAssert.fail(keyword, string or Error) from production code;MOZ_RUNTIME_ASSERT(keyword, string);Assert.whitelist.expected(keyword, regexp) or Assert.whitelist.FIXME(keyword, regexp).
//
// Module
//
let MyModule = {
oldAPI: function(foo) {
RuntimeAssert.fail(“Deprecation”, “Please use MyModule.newAPI instead of MyModule.oldAPI”);
// ...
},
newAPI: function(foo) {
// ...
},
};
let MyModule2 = {
api: function() {
return somePromise().then(null, error => {
RuntimeAssert.fail(“MyModule2.api”, error);
// Rather than leaving this error uncaught, let’s make it actionable.
});
},
api2: function(date) {
if (typeof date == “number”) {
RuntimeAssert.fail(“MyModule2.api2”, “Passing a number has been deprecated, please pass a Date”);
date = new Date(date);
}
// ...
}
}
//
// Whitelisting a RuntimeAssert in a test.
//
// This entire test is about MyModule.oldAPI, warnings are normal.
Assert.whitelist.expected(“Deprecation”, /Please use MyModule.newAPI/);
// We haven’t fixed all calls to MyModule2.api2, so they should still warn, but not cause an orange.
Assert.whitelist.FIXME(“MyModule2.api2”, /please pass a Date/);
Assert.whitelist.expected(“MyModule2.api”, /TypeError/, function() {
// In this test, we will trigger a TypeError in MyModule2.api, that’s entirely expected.
// Ignore such errors within the (async) scope of this function.
});
In the long-term, I believe that RuntimeAssert (or some other mechanism) should replace almost all our calls to Cu.reportError.
In the short-term, I plan to use this for reporting
| Комментировать | « Пред. запись — К дневнику — След. запись » | Страницы: [1] [Новые] |