Re: Standardized assertion methods
On Fri, Jun 6, 2014 at 8:35 PM, Boris Zbarsky bzbar...@mit.edu wrote: https://tbpl.mozilla.org/?tree=Tryrev=e26ab6d5e1e0 says we have quite a number of things that are in fact assuming that 5 and 5 should test is(). I'm not sure how much I like throwing in tons of toString() for that case. Why should you need .toString()? Just change the expected result. Like here, http://dxr.mozilla.org/mozilla-central/source/dom/base/test/test_url_empty_port.html#27 is(url.port, 8080, 'URL.port is 8080'); should just become is(url.port, '8080', 'URL.port is 8080'); This is something I could look at fixing when I next have time, if no one else gets to it first. Should be a lot less work than fixing every misuse of nsresult and nsnull in the tree. :) ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 07/06/2014 03:40, Ehsan Akhgari wrote: On 2014-06-06, 4:11 PM, Boris Zbarsky wrote: On 6/6/14, 3:19 PM, Ehsan Akhgari wrote: Can we make is() do those checks explicitly and if neither of these cases apply, fall back to a non-strict equality check? Yes. As in, we could make it special-case the number-to-string compare and use == for that, and use === for everything else. r=me :-) Indeed. I'm happy to review and/or help write patches for tests that still need updating because of other reasons (of which there still seem to be a few, like the location == string thing). ~ Gijs ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On Friday 2014-06-06 17:42 -0700, Jonas Sicking wrote: I definitely understand that it'll be a pain to convert existing tests that rely on the relaxed matching. But rather than making the implementation of is() be more complex and/or more relaxed, could we instead convert those tests to either is_relaxed(a, b) or ok(a == b) Ideally it seems like we want to convert the existing tests to test the correct equality, but that requires some knowledge of what they're supposed to be testing, and also somewhat more manual changes to the tests. Temporarily converting them to is_relaxed (or maybe a scarier sounding name, like deprecated_relaxed_is) might help in fixing the 99% case so that we don't add more problems on top of our existing ones, though. -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: Standardized assertion methods
On Wednesday 2014-06-04 14:10 -0400, Boris Zbarsky wrote: For example, should mochitest-plain be consistent with mochitest-chrome? I would argue yes; the distinction between which tests go in which one is more or less arbitrarily decided by what APIs we do or don't have on SpecialPowers. Maybe the right answer is that pure-JS tests like xpcshell and mochitest-browser should be consistent and combined JS-and-markup tests like mochitest-plain and mochitest-chrome should (separately) be consistent but we shouldn't worry about consistency between the two groups of tests? I would very much welcome us actually having this discussion and deciding _this_ point first, before we start deciding which APIs should be deprecated and which should be preferred in which test harness. I strongly agree here about wanting consistency between mochitest-plain and mochitest-chrome because the distinction between them is very small. As a somewhat unusually but actually probably not all that extreme example, when writing content/canvas/test/test_drawWindow.html, I realized most of the way through writing the test -- i.e., when figuring out why the test I'd written didn't show the without-patch failures that were expected -- that half of the test needed to move to mochitest-chrome. So there are now two files, one in mochitest-plain and the other (content/canvas/test/chrome/test_drawWindow_widget_layers.html) in mochitest-chrome that share between them a common JS file containing the actual test assertions. (I believe I didn't want to move the whole test, because, if my memory is correct (I'm writing this offline), we run mochitest-chrome on fewer platforms than mochitest-plain.) -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: Standardized assertion methods
On 05/06/14 10:38, Mike de Boer wrote: As I tried to explain, the CommonJS API naively made sense to me at the time. To others as well, because we’re happily using it. As I now understand, some of us are very attached to a specific, different, API. FWIW I don't think that I am attached to a specific different api. I am, however, attached to api semantics that make writing good tests easy. I don't think that either CommonJS or SimpleTest achieve this in their current form. For SimpltTest I think the main problems are: * The is() and isnot methods use non-strict equality. * ok() coerces its argument (this is a more minor problem). * is and ok seem like pretty uninformative names. ise is even worse. * isenot doesn't even exist. * The API is largely undocumented. From reading MDN you would think that is, isnot and ok were the only methods. I can't find any other doumentation except for the source. * The API seems to be inconsistently exposed e.g. doesThrow isn't placed in the global scope but is on the SimpleTest object. But it seems like some properties of SimpleTest that look like assertions are not e.g. isa seems to return a bool rather than call ok(). * doesThrow does't provide any means of inspecting the object that was thrown. * isDeeply uses non-strict equality comparisons. * All the todo stuff is mixing concerns. It forces you into a mode of test writing where properties of a single implementation are hardcoded into the testcases. This isn't a huge problem when there is only a single relevant implementation, but we do a lot of work on standards where there are multiple implementations. * The fact that the implementation to todo* has to duplicate all the comparison code is pretty terrible. Maybe that's why all the methods other than ok, is, and isn are undocumented, because they don't have todo equivalents. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On Fri, Jun 6, 2014 at 2:29 AM, James Graham ja...@hoppipolla.co.uk wrote: * All the todo stuff is mixing concerns. It forces you into a mode of test writing where properties of a single implementation are hardcoded into the testcases. This isn't a huge problem when there is only a single relevant implementation, but we do a lot of work on standards where there are multiple implementations. I agree with this. Ideally you would separate the test from the these are the parts of the test we expect to fail. In practice I think this is somewhat hard though. For simple tests it'd be possible to load separate data which indicates we currently consistently fail assertion 5, 7 and 12, and randomly fail assertion 4. But tests that perform thousands of assertions due to testing lots of combinations of features, this quickly gets hard to maintain and to update. Even worse is tests that perform an inconsistent number of assertions due to timing issues (for example tests of progress events). If we make this part of writing tests, that significantly ups the cost of writing tests, which is something we all seem to agree is one of the most important thing to avoid. I'd love to find a solution to this. In the meantime flipping todo()s into is()s when we send tests to W3C, and flipping is()s into todo()s when we import tests from W3C seems like a low-cost solution. / Jonas ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 06/06/2014 10:29, James Graham wrote: On 05/06/14 10:38, Mike de Boer wrote: As I tried to explain, the CommonJS API naively made sense to me at the time. To others as well, because we’re happily using it. As I now understand, some of us are very attached to a specific, different, API. FWIW I don't think that I am attached to a specific different api. I am, however, attached to api semantics that make writing good tests easy. I don't think that either CommonJS or SimpleTest achieve this in their current form. For SimpltTest I think the main problems are: * The is() and isnot methods use non-strict equality. I will go ahead and assert that if you have a test that relies on strict versus non-strict equality, you should be using type checks to make that explicit, not an extra '=' in your comparisons. Makes assumptions much more explicit (this return value should be a string '5' and not a number 5) rather than implied by the comparison function. IOW, I wouldn't consider this a bug. * ok() coerces its argument (this is a more minor problem). I would even say s/more minor problem/feature/. I've lost count of the number of times I've done: ok(document.getElementById(foo), Foo should exist in the document) and I'd hate to have to prepend !! to everything I wanted to check with ok(). * is and ok seem like pretty uninformative names. ise is even worse. Agreed on 'ise', but I never use that (see above). I think 'is' and 'ok' are actually great names. They are short, the meaning is intuitively fairly obvious, and they just work. * The API is largely undocumented. From reading MDN you would think that is, isnot and ok were the only methods. I can't find any other doumentation except for the source. This is fixable, of course. Much more easily so than swapping our assertion frameworks! * All the todo stuff is mixing concerns. It forces you into a mode of test writing where properties of a single implementation are hardcoded into the testcases. This isn't a huge problem when there is only a single relevant implementation, but we do a lot of work on standards where there are multiple implementations. And equally, we do a lot of work on e.g. browser mochitests. I would hate having to mess about to insert a todo() - these tests will only ever run against Firefox, and there's no point abstracting stuff. * The fact that the implementation to todo* has to duplicate all the comparison code is pretty terrible. Maybe that's why all the methods other than ok, is, and isn are undocumented, because they don't have todo equivalents. This sounds like something we can fix in the code with one more level of indirection. :-) ~ Gijs ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 06/06/14 11:41, Gijs Kruitbosch wrote: On 06/06/2014 10:29, James Graham wrote: On 05/06/14 10:38, Mike de Boer wrote: As I tried to explain, the CommonJS API naively made sense to me at the time. To others as well, because we’re happily using it. As I now understand, some of us are very attached to a specific, different, API. FWIW I don't think that I am attached to a specific different api. I am, however, attached to api semantics that make writing good tests easy. I don't think that either CommonJS or SimpleTest achieve this in their current form. For SimpltTest I think the main problems are: * The is() and isnot methods use non-strict equality. I will go ahead and assert that if you have a test that relies on strict versus non-strict equality, you should be using type checks to make that explicit, not an extra '=' in your comparisons. Makes assumptions much more explicit (this return value should be a string '5' and not a number 5) rather than implied by the comparison function. IOW, I wouldn't consider this a bug. So you believe that every single time you write is(some_func(), 5) you should also write is(typeof some_func(), number) ? That seems pretty much insane to me and I will happily assert that no one actually does it consistently. If there are cases where you really don't care about the type — and I can't think of very many — then in those cases you should explicitly type convert as a signal that you are doing something strange. * ok() coerces its argument (this is a more minor problem). I would even say s/more minor problem/feature/. I've lost count of the number of times I've done: ok(document.getElementById(foo), Foo should exist in the document) So what you are checking for there is !== null, not is a thing that coerces to true. But like I said this is a more minor concern. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 06/06/2014 11:56, James Graham wrote: On 06/06/14 11:41, Gijs Kruitbosch wrote: On 06/06/2014 10:29, James Graham wrote: On 05/06/14 10:38, Mike de Boer wrote: As I tried to explain, the CommonJS API naively made sense to me at the time. To others as well, because we’re happily using it. As I now understand, some of us are very attached to a specific, different, API. FWIW I don't think that I am attached to a specific different api. I am, however, attached to api semantics that make writing good tests easy. I don't think that either CommonJS or SimpleTest achieve this in their current form. For SimpltTest I think the main problems are: * The is() and isnot methods use non-strict equality. I will go ahead and assert that if you have a test that relies on strict versus non-strict equality, you should be using type checks to make that explicit, not an extra '=' in your comparisons. Makes assumptions much more explicit (this return value should be a string '5' and not a number 5) rather than implied by the comparison function. IOW, I wouldn't consider this a bug. So you believe that every single time you write is(some_func(), 5) you should also write is(typeof some_func(), number) ? That seems pretty much insane to me and I will happily assert that no one actually does it consistently. If there are cases where you really don't care about the type — and I can't think of very many — then in those cases you should explicitly type convert as a signal that you are doing something strange. No, I think that in 99.99% of cases, I don't care about the type, and therefore I would normally use is() and not care that it's using non-strict equality. I think the case where there is (a) a possibility that I could get '5' instead of 5 when code is malfunctioning, and (b) that would be a bug, is extremely rare, and therefore that extremely rare case should require the additional code, instead of requiring extra care on the part of test-writers to get their test to be strict about types *all the time*. ~ Gijs ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On Fri, Jun 6, 2014 at 11:12 PM, Gijs Kruitbosch gijskruitbo...@gmail.com wrote: No, I think that in 99.99% of cases, I don't care about the type, and therefore I would normally use is() and not care that it's using non-strict equality. I think the case where there is (a) a possibility that I could get '5' instead of 5 when code is malfunctioning, and (b) that would be a bug, is extremely rare, and therefore that extremely rare case should require the additional code, instead of requiring extra care on the part of test-writers to get their test to be strict about types *all the time*. I agree. I've written a lot of mochitests over many years and I have never once thought about the precise semantics of is. Nor have I ever encountered a bug that failed to be caught due to is behaving unexpectedly. I'm sure there are some kinds of tests where that matters, but for my DOM/layout tests it doesn't seem to. Rob -- Jtehsauts tshaei dS,o n Wohfy Mdaon yhoaus eanuttehrotraiitny eovni le atrhtohu gthot sf oirng iyvoeu rs ihnesa.rt sS?o Whhei csha iids teoa stiheer :p atroa lsyazye,d 'mYaonu,r sGients uapr,e tfaokreg iyvoeunr, 'm aotr atnod sgaoy ,h o'mGee.t uTph eann dt hwea lmka'n? gBoutt uIp waanndt wyeonut thoo mken.o w ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 6/6/14, 7:12 AM, Gijs Kruitbosch wrote: No, I think that in 99.99% of cases, I don't care about the type, and therefore I would normally use is() and not care that it's using non-strict equality. I think the case where there is (a) a possibility that I could get '5' instead of 5 when code is malfunctioning, and (b) that would be a bug, is extremely rare The common cases where this would be a bug are not 5 vs '5'. They're null vs undefined (the most common; we've accidentally exposed APIs when we thought we had a test for them not being exposed because of this one!) or 0 vs /null/undefined. and therefore that extremely rare case should require the additional code The problem is, the rare cases are the ones test writers never think about. The is() using == thing has bitten us far too many times. I'm just going to work on getting it fixed. -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 06/06/2014 15:26, Boris Zbarsky wrote: On 6/6/14, 7:12 AM, Gijs Kruitbosch wrote: No, I think that in 99.99% of cases, I don't care about the type, and therefore I would normally use is() and not care that it's using non-strict equality. I think the case where there is (a) a possibility that I could get '5' instead of 5 when code is malfunctioning, and (b) that would be a bug, is extremely rare The common cases where this would be a bug are not 5 vs '5'. They're null vs undefined (the most common; we've accidentally exposed APIs when we thought we had a test for them not being exposed because of this one!) or 0 vs /null/undefined. I'm interested in the API exposure case (bug link please? :-) ). I would have assumed that you would test that kind of thing by checking if the relevant API property is enumerable from the object, rather than checking for its value, which wouldn't be liable to issues with strict/non-strict semantics if the property happened to be there but 0//null. Put another way, I think that purposefully designing an API where a return value (or existing property value) has a meaningful distinction between 0, , null and/or undefined is essentially a footgun that will not just hit testwriters but also web authors, and we should avoid such intricacies if at all possible. (I recognize that we have to live, to a certain degree, with an existing set of web APIs where that is sometimes the case, but I would expect those edgecases to be covered by typeof rather than strict equality checks where possible (hello, null and new String() objects...) to make these intricacies as obvious as possible). and therefore that extremely rare case should require the additional code The problem is, the rare cases are the ones test writers never think about. The is() using == thing has bitten us far too many times. I'm just going to work on getting it fixed. I expect that you tend to write mochitest-plain tests for web APIs. Those aren't the tests I write the most, which are mochitest-browser or mochitest-chrome. I can't think of a single case where I care(d) about the null/undefined//0 distinction in those tests, and that is what my comment is based on. Tests I write tend to be based on click X, then check if Y happens, rather than poke interface X and see if it returns Y, which probably leads to different expectations of what tests test and how they use these comparison functions. It's also interesting that your experience is apparently very different from roc, who explicitly called out DOM/layout tests as also not needing these. :-) I guess that if this is actively biting us in terms of providing wrong test results, perhaps that should weigh heavier than the concern for ease-of-testing of other code, although I expect that changing the semantics of these functions after the fact might be an Everest-scale uphill battle in terms of test failures. :-( ~ Gijs ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 6/6/14, 11:04 AM, Gijs Kruitbosch wrote: I'm interested in the API exposure case (bug link please? :-) I'd have to search, sorry. :( I would have assumed that you would test that kind of thing by checking if the relevant API property is enumerable from the object, rather than checking for its value I would too, but that's not what people put in the test. Note that at least one of the APIs involved was actually defining the property always but returning null vs undefined in some cases based on permissions or whatnot (this was not a WebIDL API at that point). Put another way, I think that purposefully designing an API where a return value (or existing property value) has a meaningful distinction between 0, , null and/or undefined is essentially a footgun that will not just hit testwriters but also web authors I agree, sure. We shouldn't be doing that. I expect that you tend to write mochitest-plain tests for web APIs. Indeed. Those aren't the tests I write the most, which are mochitest-browser or mochitest-chrome. I can't think of a single case where I care(d) about the null/undefined//0 distinction in those tests, and that is what my comment is based on. Sure. But would it cause problems if we did enforce the distinction? I'll have to see how the try run looks. ;) It's also interesting that your experience is apparently very different from roc, who explicitly called out DOM/layout tests as also not needing these. :-) Yeah, I've been dealing a lot with edge cases of DOM APIs for the last year or two I guess that if this is actively biting us in terms of providing wrong test results, perhaps that should weigh heavier than the concern for ease-of-testing of other code, although I expect that changing the semantics of these functions after the fact might be an Everest-scale uphill battle in terms of test failures. :-( That will be the big question, yes. -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 6/6/14, 12:21 PM, Boris Zbarsky wrote: Yeah, I've been dealing a lot with edge cases of DOM APIs for the last year or two Case in point. I got some try results, and here's one of the failures: 748 INFO TEST-UNEXPECTED-FAIL | /tests/dom/indexedDB/test/test_multientry.html | exhausted indexes - got null, expected undefined The test says: 93 is(req.result, undefined, exhausted indexes); The spec says: If performing operation failed then revert all changes made by operation, set the done flag on the request to true, set result of the request to undefined So we're not following the spec, and the test that is supposedly supposed to check that is incorrectly claiming we are... -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 6/6/14, 12:21 PM, Boris Zbarsky wrote: That will be the big question, yes. https://tbpl.mozilla.org/?tree=Tryrev=e26ab6d5e1e0 says we have quite a number of things that are in fact assuming that 5 and 5 should test is(). I'm not sure how much I like throwing in tons of toString() for that case. There are also tests that explicitly compare 0.40 with 0.4 and expect that to pass. I didn't look to see whether the formatting of the 0.40 was meaningful or the result of using toFixed() to get rounding. We also have the following things that I think should in fact be fixed no matter what we do with number vs string: 1) null vs undefined 2) Location objects vs strings. 3) DOMTokenList objects vs strings 4) DOM nodes vs stuff like [object HTMLDivElement] (does the test really not care _which_ div it got??) 5) Arrays vs strings (e.g. is([foo], foo) 6) Date objects vs strings 7) 0 vs false (in bc1, complete with unknown test url, yay). 8) Various objects vs strings (akin to #4 but for Xray wrappers and whatnot). -boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 2014-06-05, 2:08 AM, Boris Zbarsky wrote: On 6/4/14, 11:30 PM, Gavin Sharp wrote: - benefits to shared API/implementation seem uncontroversial Agreed. - specifically, consistency between mochitest/SimpleTest-based harnesses (mochitest-plain/mochitest-chrome/mochitest-browser) and xpcshell tests is what we care about primarily. I don't think we should expand the scope of this discussion beyond that to e.g. testharness.js. That works for me. - I'm not particularly attached to CommonJS assertions, and probably actually favor using the existing SimpleTest API as the one true test assertion API since it's more familiar to me and seems to be working well. Sounds like bz/ehsan/sicking all agree I do. Assuming agreement across those, that suggests we should modify Assert.jsm to use the SimpleTest API instead, and continue with this plan to gradually encourage use of Assert.jsm for the mochitest-based harnesses and xpcshell. If it's using the SimpleTest API, we could easily, and should, just switch the mochitest-based harnesses to it wholesale. Agreed on all of the above. (Separately, we could then perhaps address any shortcomings with the existing SimpleTest assertions Top of the list: make is() do a === instead of == and get rid of ise(). I'd hope this can actually be done without too many cycles through try... We only have 237 ise calls, so at least the latter should be fairly simple. The number of things which would fail if is() does a === is yet to be known before someone starts trying... Cheers, Ehsan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 2014-06-06, 1:35 PM, Boris Zbarsky wrote: On 6/6/14, 12:21 PM, Boris Zbarsky wrote: That will be the big question, yes. https://tbpl.mozilla.org/?tree=Tryrev=e26ab6d5e1e0 says we have quite a number of things that are in fact assuming that 5 and 5 should test is(). I'm not sure how much I like throwing in tons of toString() for that case. There are also tests that explicitly compare 0.40 with 0.4 and expect that to pass. I didn't look to see whether the formatting of the 0.40 was meaningful or the result of using toFixed() to get rounding. We also have the following things that I think should in fact be fixed no matter what we do with number vs string: 1) null vs undefined 2) Location objects vs strings. 3) DOMTokenList objects vs strings 4) DOM nodes vs stuff like [object HTMLDivElement] (does the test really not care _which_ div it got??) 5) Arrays vs strings (e.g. is([foo], foo) 6) Date objects vs strings 7) 0 vs false (in bc1, complete with unknown test url, yay). 8) Various objects vs strings (akin to #4 but for Xray wrappers and whatnot). Can we make is() do those checks explicitly and if neither of these cases apply, fall back to a non-strict equality check? ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 6/6/14, 3:19 PM, Ehsan Akhgari wrote: Can we make is() do those checks explicitly and if neither of these cases apply, fall back to a non-strict equality check? Yes. As in, we could make it special-case the number-to-string compare and use == for that, and use === for everything else. -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On Fri, Jun 6, 2014 at 1:11 PM, Boris Zbarsky bzbar...@mit.edu wrote: On 6/6/14, 3:19 PM, Ehsan Akhgari wrote: Can we make is() do those checks explicitly and if neither of these cases apply, fall back to a non-strict equality check? Yes. As in, we could make it special-case the number-to-string compare and use == for that, and use === for everything else. I don't see why we would want to do that though. We should test that we get the expected result, not approximately the expected result. Something that passes our test might very well break a real-world web page. I.e. having a function that returns 0.4, but where the spec says to return 0.4 should result in a test failure, since returning 0.4 might very well break the web. I definitely understand that it'll be a pain to convert existing tests that rely on the relaxed matching. But rather than making the implementation of is() be more complex and/or more relaxed, could we instead convert those tests to either is_relaxed(a, b) or ok(a == b) / Jonas ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 6/6/14, 8:42 PM, Jonas Sicking wrote: I definitely understand that it'll be a pain to convert existing tests that rely on the relaxed matching. Yes. We have about 3300 failing test assertions with is() using ===, and the vast majority are string-vs-number. All I'm saying is that we could allow that to keep working and tighten up is() for the other cases much faster than we can whack all the moles. ;) But rather than making the implementation of is() be more complex and/or more relaxed, could we instead convert those tests to either is_relaxed(a, b) or ok(a == b) Yes, we could. It's a largish number of tests (several hundred; a number have multiple assertions in a single test, so there are definitely many fewer than 3000 _tests_ involved). On the other hand, if you're volunteering to review patches, I'll make sure they get written... ;) -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On Fri, Jun 6, 2014 at 7:10 PM, Boris Zbarsky bzbar...@mit.edu wrote: On 6/6/14, 8:42 PM, Jonas Sicking wrote: I definitely understand that it'll be a pain to convert existing tests that rely on the relaxed matching. Yes. We have about 3300 failing test assertions with is() using ===, and the vast majority are string-vs-number. All I'm saying is that we could allow that to keep working and tighten up is() for the other cases much faster than we can whack all the moles. ;) If that counts as low-hanging fruit then that's certainly a step in the right direction. The question is what to do as step 2. But rather than making the implementation of is() be more complex and/or more relaxed, could we instead convert those tests to either is_relaxed(a, b) or ok(a == b) Yes, we could. It's a largish number of tests (several hundred; a number have multiple assertions in a single test, so there are definitely many fewer than 3000 _tests_ involved). On the other hand, if you're volunteering to review patches, I'll make sure they get written... ;) I'm not sure that I'm prepared to take them on all by myself :) But I can certainly help. / Jonas ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 2014-06-06, 4:11 PM, Boris Zbarsky wrote: On 6/6/14, 3:19 PM, Ehsan Akhgari wrote: Can we make is() do those checks explicitly and if neither of these cases apply, fall back to a non-strict equality check? Yes. As in, we could make it special-case the number-to-string compare and use == for that, and use === for everything else. r=me :-) ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 2014-06-06, 10:31 PM, Jonas Sicking wrote: But rather than making the implementation of is() be more complex and/or more relaxed, could we instead convert those tests to either is_relaxed(a, b) or ok(a == b) Yes, we could. It's a largish number of tests (several hundred; a number have multiple assertions in a single test, so there are definitely many fewer than 3000 _tests_ involved). On the other hand, if you're volunteering to review patches, I'll make sure they get written... ;) I'm not sure that I'm prepared to take them on all by myself :) But I can certainly help. If you find someone who's willing to do this, I'll happily help review the patches. That being said, I think starting smaller as you suggested earlier and gradually (perhaps developing in the try server) moving towards the more noble goal makes more sense to me. Cheers, Ehsan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 6/4/14, 11:30 PM, Gavin Sharp wrote: - benefits to shared API/implementation seem uncontroversial Agreed. - specifically, consistency between mochitest/SimpleTest-based harnesses (mochitest-plain/mochitest-chrome/mochitest-browser) and xpcshell tests is what we care about primarily. I don't think we should expand the scope of this discussion beyond that to e.g. testharness.js. That works for me. - I'm not particularly attached to CommonJS assertions, and probably actually favor using the existing SimpleTest API as the one true test assertion API since it's more familiar to me and seems to be working well. Sounds like bz/ehsan/sicking all agree I do. Assuming agreement across those, that suggests we should modify Assert.jsm to use the SimpleTest API instead, and continue with this plan to gradually encourage use of Assert.jsm for the mochitest-based harnesses and xpcshell. If it's using the SimpleTest API, we could easily, and should, just switch the mochitest-based harnesses to it wholesale. (Separately, we could then perhaps address any shortcomings with the existing SimpleTest assertions Top of the list: make is() do a === instead of == and get rid of ise(). I'd hope this can actually be done without too many cycles through try... -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 04.06.2014 11:45, Mike de Boer wrote: The reason CommonJS came into view was not because of it’s semantic superiority, but because of its similarity to both the XPCShell-test and Mochitest assertion styles and implementation. This way I thought we could circumvent ppl to get worried about re-inventing the wheel or something like that and view this change as an incremental step to gradually improve the blueprint overlap between the test suites in use. I don't understand. How does using CommonJS achieve this better than making XPCShell use something based on the Mochitest API? Dao ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 05.06.2014 09:54, Dao wrote: On 04.06.2014 11:45, Mike de Boer wrote: The reason CommonJS came into view was not because of it’s semantic superiority, but because of its similarity to both the XPCShell-test and Mochitest assertion styles and implementation. This way I thought we could circumvent ppl to get worried about re-inventing the wheel or something like that and view this change as an incremental step to gradually improve the blueprint overlap between the test suites in use. I don't understand. How does using CommonJS achieve this better than making XPCShell use something based on the Mochitest API? And by Mochitest API I meant SimpleTest, I guess... ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 05 Jun 2014, at 09:54, Dao d...@design-noir.de wrote: On 04.06.2014 11:45, Mike de Boer wrote: The reason CommonJS came into view was not because of it’s semantic superiority, but because of its similarity to both the XPCShell-test and Mochitest assertion styles and implementation. This way I thought we could circumvent ppl to get worried about re-inventing the wheel or something like that and view this change as an incremental step to gradually improve the blueprint overlap between the test suites in use. I don't understand. How does using CommonJS achieve this better than making XPCShell use something based on the Mochitest API? It doesn’t, per sé. Please understand that I’m not at all attached to _any_ API. I care only about pragmatic consistency across test suites we use for frontend development. If possible, across the board. As I tried to explain, the CommonJS API naively made sense to me at the time. To others as well, because we’re happily using it. As I now understand, some of us are very attached to a specific, different, API. I care only about the sanity of its implementation. The problems cited by James and echoed by Boris concerning `deepEqual()` are thusly most important to me[1]. Renaming `strictEqual` to `equal`, nuking `strictEqual` from orbit, is more than fine by me. Or we name it `is`. (As an aside, whilst maintaining my position of not caring about it, I don’t understand why ‘we’ like an ambiguous term `is` better than `equal`.) Mike. [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1020875 ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 05.06.2014 11:38, Mike de Boer wrote: On 05 Jun 2014, at 09:54, Dao d...@design-noir.de wrote: On 04.06.2014 11:45, Mike de Boer wrote: The reason CommonJS came into view was not because of it’s semantic superiority, but because of its similarity to both the XPCShell-test and Mochitest assertion styles and implementation. This way I thought we could circumvent ppl to get worried about re-inventing the wheel or something like that and view this change as an incremental step to gradually improve the blueprint overlap between the test suites in use. I don't understand. How does using CommonJS achieve this better than making XPCShell use something based on the Mochitest API? It doesn’t, per sé. Please understand that I’m not at all attached to _any_ API. I care only about pragmatic consistency across test suites we use for frontend development. If possible, across the board. That's a fine goal and apparently generally agreed with (except for testharness.js). As I tried to explain, the CommonJS API naively made sense to me at the time. To others as well, because we’re happily using it. As I now understand, some of us are very attached to a specific, different, API. While I like that the SimpleTest methods tend to be briefer, I don't think the main issue is that people are deeply attached to that API per se. The point is that we shouldn't replace it without good reasons, since there's a cost attached to switching APIs. If we were using CommonJS and someone proposed we moved to SimpleTest, there would likely be a similar backlash. I care only about the sanity of its implementation. The problems cited by James and echoed by Boris concerning `deepEqual()` are thusly most important to me[1]. Renaming `strictEqual` to `equal`, nuking `strictEqual` from orbit, is more than fine by me. Or we name it `is`. (As an aside, whilst maintaining my position of not caring about it, I don’t understand why ‘we’ like an ambiguous term `is` better than `equal`.) This is the first time I hear the complaint that 'is' would be ambiguous. This isn't a problem that occurred to me while writing and reviewing tests over the years. Dao ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 05 Jun 2014, at 12:00, Dao d...@design-noir.de wrote: On 05.06.2014 11:38, Mike de Boer wrote: On 05 Jun 2014, at 09:54, Dao d...@design-noir.de wrote: On 04.06.2014 11:45, Mike de Boer wrote: The reason CommonJS came into view was not because of it’s semantic superiority, but because of its similarity to both the XPCShell-test and Mochitest assertion styles and implementation. This way I thought we could circumvent ppl to get worried about re-inventing the wheel or something like that and view this change as an incremental step to gradually improve the blueprint overlap between the test suites in use. I don't understand. How does using CommonJS achieve this better than making XPCShell use something based on the Mochitest API? It doesn’t, per sé. Please understand that I’m not at all attached to _any_ API. I care only about pragmatic consistency across test suites we use for frontend development. If possible, across the board. I actually meant ‘test runners’, not ‘test suites’. But we already understood each other :) That's a fine goal and apparently generally agreed with (except for testharness.js). As I tried to explain, the CommonJS API naively made sense to me at the time. To others as well, because we’re happily using it. As I now understand, some of us are very attached to a specific, different, API. While I like that the SimpleTest methods tend to be briefer, I don't think the main issue is that people are deeply attached to that API per se. The point is that we shouldn't replace it without good reasons, since there's a cost attached to switching APIs. If we were using CommonJS and someone proposed we moved to SimpleTest, there would likely be a similar backlash. I’m a bit wary of the ‘cost’ propositions I hear every now and again. Everything we do has a cost. Having inconsistent APIs across the codebase has a cost. Having this conversation has a cost. Having an assertion method called `ise` has a cost. I’m not sure what you’re trying to discuss here. I want that to happen, which is most ‘cost-efficient’; if that’s moving the implementation of `is`, `isnot`, `ise` to a separate module whilst keeping the method names available to all Mochitest(-browser) tests, than we do that. If providing the SimpleTest API to XPCShell-test by aliasing the assertion method names to the respective SimpleTest method names and not exposing the CommonJS method names in the global scope is most ‘cost-efficient’, than we do that. I care only about the sanity of its implementation. The problems cited by James and echoed by Boris concerning `deepEqual()` are thusly most important to me[1]. Renaming `strictEqual` to `equal`, nuking `strictEqual` from orbit, is more than fine by me. Or we name it `is`. (As an aside, whilst maintaining my position of not caring about it, I don’t understand why ‘we’ like an ambiguous term `is` better than `equal`.) This is the first time I hear the complaint that 'is' would be ambiguous. This isn't a problem that occurred to me while writing and reviewing tests over the years. Well, I consider reading it out load in English: is(foo, bar); Should it read is foo [?] to bar?” or “is foo [equal to] bar?” or “is foo [strict equal to] bar?” or “… foo [is] bar?” equal(foo, bar); Should it read “Are foo and bar equal?”. Mike. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On Thu, Jun 5, 2014 at 3:33 AM, Mike de Boer mdeb...@mozilla.com wrote: I want that to happen, which is most ‘cost-efficient’; if that’s moving the implementation of `is`, `isnot`, `ise` to a separate module whilst keeping the method names available to all Mochitest(-browser) tests, than we do that. If providing the SimpleTest API to XPCShell-test by aliasing the assertion method names to the respective SimpleTest method names and not exposing the CommonJS method names in the global scope is most ‘cost-efficient’, than we do that. Sounds like we have a clear path forward: adjust Assert.jsm's API to match the SimpleTest API, then continue moving forward with the plan to use Assert.jsm across mochitest/xpcshell (and deprecate the old xpcshell assertion methods). Shall we get some bugs on file? Gavin ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 04 Jun 2014, at 00:33, James Graham ja...@hoppipolla.co.uk wrote: On 03/06/14 20:34, Boris Zbarsky wrote: I'm arguing against Assert.jsm using the commonjs API names. And I am arguing against using the CommonJS semantics. If we are adding new assertions it shouldn't be ones that encourage broken tests. I think this is very subjective and, to be honest, the first time I heard someone say that the CommonJS semantics are broken, even encourage broken tests. The API surface is concise enough to limit the amount of typing and convey the meaning of the method used. They achieved this to closely follow the English verbs of operators used to test an code block. I really don’t see how much closer you’d like to get to 'doing what you say you’re going to do' as far as API semantics go. I realise that this reasoning is subjective too. Furthermore, are the tests we have currently broken? Is there something we need to get increasingly worried about? The reason CommonJS came into view was not because of it’s semantic superiority, but because of its similarity to both the XPCShell-test and Mochitest assertion styles and implementation. This way I thought we could circumvent ppl to get worried about re-inventing the wheel or something like that and view this change as an incremental step to gradually improve the blueprint overlap between the test suites in use. The benefit, IMHO, of incidentally using an assertion API that is widely used by the Javascript community, globally, was a nice side-effect. As a disclaimer, I’m by no means a fan of the CommonJS group itself. I would even go as far as to say that this particular spec is the only one that might survive the sands of time. Mike. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 2014-06-04, 5:45 AM, Mike de Boer wrote: On 04 Jun 2014, at 00:33, James Graham ja...@hoppipolla.co.uk wrote: On 03/06/14 20:34, Boris Zbarsky wrote: I'm arguing against Assert.jsm using the commonjs API names. And I am arguing against using the CommonJS semantics. If we are adding new assertions it shouldn't be ones that encourage broken tests. I think this is very subjective and, to be honest, the first time I heard someone say that the CommonJS semantics are broken, even encourage broken tests. The API surface is concise enough to limit the amount of typing and convey the meaning of the method used. They achieved this to closely follow the English verbs of operators used to test an code block. I really don’t see how much closer you’d like to get to 'doing what you say you’re going to do' as far as API semantics go. I realise that this reasoning is subjective too. Furthermore, are the tests we have currently broken? Is there something we need to get increasingly worried about? Define broken. We do have quirks in each of our test frameworks that we'd do differently if we went back in time and wanted to redo things again. The reason CommonJS came into view was not because of it’s semantic superiority, but because of its similarity to both the XPCShell-test and Mochitest assertion styles and implementation. This way I thought we could circumvent ppl to get worried about re-inventing the wheel or something like that and view this change as an incremental step to gradually improve the blueprint overlap between the test suites in use. The benefit, IMHO, of incidentally using an assertion API that is widely used by the Javascript community, globally, was a nice side-effect. Well, honestly it seems like we're approaching the problem backwards, by picking CommonJS first, and then figuring out what the implications are. I think the right way to approach this is the exact opposite: first define what the problems that we're trying to solve are, then figure out what the technical options on solving those are, whether they require *any* changes to the test APIs, and if so try to justify the changes very well, get feedback on the proposed goals and changes to satisfy them, rinse and repeat. The other thing that you need to note is that there is years of experience behind each one of our test frameworks, and there are probably several hundred thousand lines of code written against any of them. And there are many many people who have been using these frameworks for years now. Completely replacing the testing API is pretty much as major a change as one could possibly imagine. This is not a gradual change in any way. Cheers, Ehsan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 04 Jun 2014, at 19:20, Ehsan Akhgari ehsan.akhg...@gmail.com wrote: On 2014-06-04, 5:45 AM, Mike de Boer wrote: On 04 Jun 2014, at 00:33, James Graham ja...@hoppipolla.co.uk wrote: On 03/06/14 20:34, Boris Zbarsky wrote: I'm arguing against Assert.jsm using the commonjs API names. And I am arguing against using the CommonJS semantics. If we are adding new assertions it shouldn't be ones that encourage broken tests. I think this is very subjective and, to be honest, the first time I heard someone say that the CommonJS semantics are broken, even encourage broken tests. The API surface is concise enough to limit the amount of typing and convey the meaning of the method used. They achieved this to closely follow the English verbs of operators used to test an code block. I really don’t see how much closer you’d like to get to 'doing what you say you’re going to do' as far as API semantics go. I realise that this reasoning is subjective too. Furthermore, are the tests we have currently broken? Is there something we need to get increasingly worried about? Define broken. We do have quirks in each of our test frameworks that we'd do differently if we went back in time and wanted to redo things again. I wasn’t implying that they’re broken at all, it’s just that James was hinting at that. The reason CommonJS came into view was not because of it’s semantic superiority, but because of its similarity to both the XPCShell-test and Mochitest assertion styles and implementation. This way I thought we could circumvent ppl to get worried about re-inventing the wheel or something like that and view this change as an incremental step to gradually improve the blueprint overlap between the test suites in use. The benefit, IMHO, of incidentally using an assertion API that is widely used by the Javascript community, globally, was a nice side-effect. Well, honestly it seems like we're approaching the problem backwards, by picking CommonJS first, and then figuring out what the implications are. I think the right way to approach this is the exact opposite: first define what the problems that we're trying to solve are, then figure out what the technical options on solving those are, whether they require *any* changes to the test APIs, and if so try to justify the changes very well, get feedback on the proposed goals and changes to satisfy them, rinse and repeat. I agree and I think that’s how things happened and the way I explained it above. As with anything that happens in the tree, they can be reversed if they appear to have been erroneous; back out. If you think that is the correct thing to do at this point, please file a bug to make this happen. The other thing that you need to note is that there is years of experience behind each one of our test frameworks, and there are probably several hundred thousand lines of code written against any of them. And there are many many people who have been using these frameworks for years now. Completely replacing the testing API is pretty much as major a change as one could possibly imagine. This is not a gradual change in any way. It could be that we have a different understanding of the topic at hand, but we are talking about assertion methods, right? They’re the simplest of wrappers around comparing ‘a' to ‘b’ with `==` or `===`. I fail to see how consolidating these methods in a standalone module is at all about throwing away years of experience. Please understand that I did NOT replace the testing API, it is still there. The changesets of the bug that cover this work are minimal and intentionally so. Everyone can continue writing XPCShell tests the way they were used to. All the existing tests work like they did before. The assertion methods are still there and I don’t think they will _ever_ be removed. I’m not even saying they should be removed. Again, the implementation of the assertions themselves are still the same as before, they just live someplace else. Call it CommonJS, call it UnCommonJS, call it anything you want; things. did. not. change. I blame this misunderstanding on my apparent inability to explain things well enough from the get-go. My apologies. Mike. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 6/4/2014 1:20 PM, Ehsan Akhgari wrote: The other thing that you need to note is that there is years of experience behind each one of our test frameworks, and there are probably several hundred thousand lines of code written against any of them. And there are many many people who have been using these frameworks for years now. Completely replacing the testing API is pretty much as major a change as one could possibly imagine. This is not a gradual change in any way. I think you're giving far too much credit to the years of experience bit. Most of our test harnesses were scavenged from somewhere else and have a pile of hacks built on top of that. The inconsistency between our various test harnesses makes it harder than necessary to write different types of tests. I think everyone can agree that consistency there is a worthy goal. Is the CommonJS assert module the perfect test assertion API? Almost certainly not, but the main advantage is that it's had *some* API design applied to it, vs. the organic growth into a monstrosity that we currently have in our test suites. It seems like a net positive change to me. Changing the entire world in one fell swoop is a huge undertaking. We've attempted to do something like this in other situations, like using SpecialPowers to make tests e10s-friendly, and it's really hard. I don't think any change ought to be blocked on that. RE: the discussion of testharness.js etc, I think those are even farther afield, since the testing model there is much different from what we have in Mochitest/xpcshell tests. It makes sense to align our web content tests with W3C testing, since that means we can share tests with other browser vendors, but we still have a huge existing base of Mochitests that aren't going to get ported to that style anytime soon. -Ted ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 2014-06-04, 1:42 PM, Mike de Boer wrote: On 04 Jun 2014, at 19:20, Ehsan Akhgari ehsan.akhg...@gmail.com mailto:ehsan.akhg...@gmail.com wrote: On 2014-06-04, 5:45 AM, Mike de Boer wrote: On 04 Jun 2014, at 00:33, James Graham ja...@hoppipolla.co.uk mailto:ja...@hoppipolla.co.uk wrote: On 03/06/14 20:34, Boris Zbarsky wrote: I'm arguing against Assert.jsm using the commonjs API names. And I am arguing against using the CommonJS semantics. If we are adding new assertions it shouldn't be ones that encourage broken tests. I think this is very subjective and, to be honest, the first time I heard someone say that the CommonJS semantics are broken, even encourage broken tests. The API surface is concise enough to limit the amount of typing and convey the meaning of the method used. They achieved this to closely follow the English verbs of operators used to test an code block. I really don’t see how much closer you’d like to get to 'doing what you say you’re going to do' as far as API semantics go. I realise that this reasoning is subjective too. Furthermore, are the tests we have currently broken? Is there something we need to get increasingly worried about? Define broken. We do have quirks in each of our test frameworks that we'd do differently if we went back in time and wanted to redo things again. I wasn’t implying that they’re broken at all, it’s just that James was hinting at that. Fair enough. I'll let James speak to that. :-) The reason CommonJS came into view was not because of it’s semantic superiority, but because of its similarity to both the XPCShell-test and Mochitest assertion styles and implementation. This way I thought we could circumvent ppl to get worried about re-inventing the wheel or something like that and view this change as an incremental step to gradually improve the blueprint overlap between the test suites in use. The benefit, IMHO, of incidentally using an assertion API that is widely used by the Javascript community, globally, was a nice side-effect. Well, honestly it seems like we're approaching the problem backwards, by picking CommonJS first, and then figuring out what the implications are. I think the right way to approach this is the exact opposite: first define what the problems that we're trying to solve are, then figure out what the technical options on solving those are, whether they require *any* changes to the test APIs, and if so try to justify the changes very well, get feedback on the proposed goals and changes to satisfy them, rinse and repeat. I agree and I think that’s how things happened and the way I explained it above. As with anything that happens in the tree, they can be reversed if they appear to have been erroneous; back out. If you think that is the correct thing to do at this point, please file a bug to make this happen. Like I've said before, I don't care that much about xpcshell-tests framework, so I have no strong objection to what has already landed... The other thing that you need to note is that there is years of experience behind each one of our test frameworks, and there are probably several hundred thousand lines of code written against any of them. And there are many many people who have been using these frameworks for years now. Completely replacing the testing API is pretty much as major a change as one could possibly imagine. This is not a gradual change in any way. It could be that we have a different understanding of the topic at hand, but we are talking about assertion methods, right? They’re the simplest of wrappers around comparing ‘a' to ‘b’ with `==` or `===`. I fail to see how consolidating these methods in a standalone module is at all about throwing away years of experience. Here's an example. We know that the way that the SimpleTest's is function works (using ==) is broken. If that is the problem that we're trying to solve, we won't need to change the test API at all. Please understand that I did NOT replace the testing API, it is still there. The changesets of the bug that cover this work are minimal and intentionally so. Everyone can continue writing XPCShell tests the way they were used to. All the existing tests work like they did before. The assertion methods are still there and I don’t think they will _ever_ be removed. I’m not even saying they should be removed. I understand that. The part I'm discussing right now is extending Assert.jsm API to mochitest-browser and mochitest-chrome, declaring the existing SimpleTest APIs there as obsolete, and requiring reviewers to enforce this for new code. Again, the implementation of the assertions themselves are still the same as before, they just live someplace else. Call it CommonJS, call it UnCommonJS, call it anything you want; things. did. not. change. I blame this misunderstanding on my apparent inability to explain things well enough from the get-go. My apologies. I don't mean to come off too negative, and please
Re: Standardized assertion methods
On 6/4/14, 1:42 PM, Mike de Boer wrote: I wasn’t implying that they’re broken at all, it’s just that James was hinting at that. Our existing testing frameworks are broken in terms of the goals of the testharness framework, as far as I understand. For example, one of the primary goals of testharness is to provide useful data about which things pass and which things fail even when the implementation doesn't pass all the tests. This includes things like uncaught exceptions marking the current test failed and continuing on to the next logical test, for example. That's because testharness is designed to be run in an environment where you can assume that the software being tested will fail some (possibly very large) fraction of the tests. This goal is at best a secondary goal for our harnesses, and for CommonJS, as far as I can tell. We typically expect our software to pass all of our tests, and do a bit of best-effort to extract useful information when it fails them, but we don't _really_ do what it would take to find all the failures. Even ignoring some of our pathological test suites which don't report anything useful at all on failure, mochitest-plain will stop a test file (which may contain multiple logical tests) at the first uncaught exception. All of which is something James cares about deeply, and something that absolutely blocks usage of CommonJS in the testharness test suite, but not the thing Ehsan and I are worrying about right this second. Again, the implementation of the assertions themselves are still the same as before, they just live someplace else. No one has a problem with that, as we've said repeatedly. But this thread started with, and I quote: Now we can say that the ‘old’ XPCShell-test assertion methods are deprecated in favour of the Assert.jsm ones. and We’re planning to do the same for Mochitest-browser tests in bug 1018226. which sure sounds like we're planning to deprecate the current assertion methods in Mochitest-browser. Is that conclusion actually a misunderstanding of the situation? I blame this misunderstanding on my apparent inability to explain things well enough from the get-go. My apologies. It's not clear to me that there's any misunderstanding here. -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 6/4/14, 1:56 PM, Ted Mielczarek wrote: The inconsistency between our various test harnesses makes it harder than necessary to write different types of tests. Yes, agreed. RE: the discussion of testharness.js etc, I think those are even farther afield, since the testing model there is much different from what we have in Mochitest/xpcshell tests. It makes sense to align our web content tests with W3C testing This necessarily requires them to be inconsistent with mochitest-browser and mochitest-chrome, right? Unless we also align those with testharness... If we're really trying to get stuff to be consistent, can we please try to decide what we're trying to make consistent with what? For example, should mochitest-plain be consistent with mochitest-chrome? I would argue yes; the distinction between which tests go in which one is more or less arbitrarily decided by what APIs we do or don't have on SpecialPowers. Maybe the right answer is that pure-JS tests like xpcshell and mochitest-browser should be consistent and combined JS-and-markup tests like mochitest-plain and mochitest-chrome should (separately) be consistent but we shouldn't worry about consistency between the two groups of tests? I would very much welcome us actually having this discussion and deciding _this_ point first, before we start deciding which APIs should be deprecated and which should be preferred in which test harness. but we still have a huge existing base of Mochitests that aren't going to get ported to that style anytime soon. And a lot of them can't be, because they rely on Gecko-specific bits of various sorts (SpecialPowers comes to mind). -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 04/06/14 18:42, Mike de Boer wrote: On 04 Jun 2014, at 19:20, Ehsan Akhgari ehsan.akhg...@gmail.com wrote: On 2014-06-04, 5:45 AM, Mike de Boer wrote: On 04 Jun 2014, at 00:33, James Graham ja...@hoppipolla.co.uk wrote: On 03/06/14 20:34, Boris Zbarsky wrote: I'm arguing against Assert.jsm using the commonjs API names. And I am arguing against using the CommonJS semantics. If we are adding new assertions it shouldn't be ones that encourage broken tests. I think this is very subjective and, to be honest, the first time I heard someone say that the CommonJS semantics are broken, even encourage broken tests. The API surface is concise enough to limit the amount of typing and convey the meaning of the method used. They achieved this to closely follow the English verbs of operators used to test an code block. I really don’t see how much closer you’d like to get to 'doing what you say you’re going to do' as far as API semantics go. I realise that this reasoning is subjective too. Furthermore, are the tests we have currently broken? Is there something we need to get increasingly worried about? Define broken. We do have quirks in each of our test frameworks that we'd do differently if we went back in time and wanted to redo things again. I wasn’t implying that they’re broken at all, it’s just that James was hinting at that. OK, there seems to be some confusion about what I believe so I will try to be as explicit as possible: The CommonJS test assertions have semantics that are problematic when writing tests. For example: * They favour (through brevity) the use of == comparisons instead of === comparisons (or SameValue comparisons) * They have function names that are ambiguous and therefore confusing (notStrictEquals). * They encourage the use of deepEqual which has underdefined semantics, particularly in the case of objects that contain cycles (it looks like Assert.jsm goes into an infinite loop in this case, but I may have misread the code). * The throws method encourages lazy testing since it doesn't provide any way to inspect the properties of the thrown exception. These concerns with semantics are irrespective of where these functions are used i.e. this is not just a concern related to testharness.js (although I would certainly not accept compatible assertions landing there for the above reasons). I think that having a common set of assertion functions in multiple harnesses is a mildly worthwhile goal, as is a shared implementation, albeit that the latter will only work within the Mozilla ecosystem. I think that compatibility with NodeJS is a non-goal, or at least no more important than compatibility with any other existing test frameworks. I don't personally share the concern with the length of the assert names, but I think this is a reasonable discussion to have. I think the argument that if we land these now we can change things in the future is troubling; we almost certainly won't be able to change the behaviour of any existing assertion function, or at least doing so will be a lot of work. And adding more and more assertions covering the same functionality, but with different semantics is only going to make things more confusing, negating the positive impact of sharing names cross-suite. Therefore, if we want to proceed with this work in order to get the benefit of shared code/api, we should start by ditching the specifics of current implementation, and design an API that actually meets all our requirements. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
I still don't believe either of you :) Obviously my position isn't let's make it it more frustrating to write tests; I think you're both vastly overstating the costs of switching to a slightly different, similar API. Any change is initially jarring, but I just don't buy that this change would cause it to be more annoying to write tests in the long term. But really that doesn't matter. Here are some high-level points we can probably agree on: - benefits to shared API/implementation seem uncontroversial - specifically, consistency between mochitest/SimpleTest-based harnesses (mochitest-plain/mochitest-chrome/mochitest-browser) and xpcshell tests is what we care about primarily. I don't think we should expand the scope of this discussion beyond that to e.g. testharness.js. - I'm not particularly attached to CommonJS assertions, and probably actually favor using the existing SimpleTest API as the one true test assertion API since it's more familiar to me and seems to be working well. Sounds like bz/ehsan/sicking all agree, and it should represent a net reduction in work (since it's somewhat more trivial to move existing tests over). Assuming agreement across those, that suggests we should modify Assert.jsm to use the SimpleTest API instead, and continue with this plan to gradually encourage use of Assert.jsm for the mochitest-based harnesses and xpcshell. (Separately, we could then perhaps address any shortcomings with the existing SimpleTest assertions, and other issues sicking identified). Gavin On Tue, Jun 3, 2014 at 12:41 PM, Bobby Holley bobbyhol...@gmail.com wrote: On Tue, Jun 3, 2014 at 12:31 PM, Boris Zbarsky bzbar...@mit.edu wrote: I can certainly buy it's longer than what I'm used to, and even incremental effort is required - just not incremental effort is required and that effort is non-negligible given other factors Purely subjectively, it's non-negligible. The more frustrating writing a test is, the less I want to do it, and the more I'll skimp on testing exhaustively... Maybe that's a personal failing of mine, but I suspect not. The way I see it, anything that makes the test-writing experience more frustrating is a bar to us having better test coverage. +1 to all of this. ___ 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
Re: Standardized assertion methods
On 03.06.2014 00:42, Ehsan Akhgari wrote: Also, I'm not sure where the original discussion happened, Ditto. For a change that affects pretty much all mozilla-central hackers, I would have expected a public proposal and a feedback round on this list. (My initial reaction was similar to Ehsan's and Boris', i.e. why not start with the SimpleTest API, maybe make adjustments here and there where strictly needed, and then have xpcshell adopt that?) ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 03/06/14 00:24, Chris Peterson wrote: On 6/2/14, 3:42 PM, Ehsan Akhgari wrote: 2. I also value consistency more than my personal preferences, and based on that, using the existing APIs in some tests and the new APIs in other tests (even if we agreed that #1 above doesn't matter) is strictly worse than the status quo. btw, in the mozilla.dev.tech.javascript-engine.internals fork of this thread, bz and David Bruant pointed out that W3C's testharness and TC39's test262 each use yet another set of assertion function names. Any tests we import from those test suites will need glue code to integrate with our test harness(es). In fact, for testharness.js tests (and the W3C web-platform-tests in general) the plan is to have a dedicated test harness (bug 945222). This is already up and running on tbpl on Cedar and will be turned on for mozilla-central as soon as the intermittents are under control (Linux is looking good, Windows has some issues with WebVTT tests, OSX shows a little more variability). As a result, in the near future we won't need glue code between testharness.js tests and other kinds of tests. FWIW I think the main problem with the CommonJS assertions is their their semantics. For example: * Test assertions shouldn't silently type-cast, but ok, equal and notEqual all do that. Their brevity compared to strictEqual and notStrictEqual means that they are likely to be much more widely used. * notStrictEqual and notDeepEqual are terrible names that actively mislead about what the functions do. * deepEqual has, as far as I can tell, underspecified semantics. I can't tell if it is supposed to recurse into nested objects and, if so, whether there is supposed to be be any loop prevention (the spec talks about equivalent values for every key, without defining what this means). * throws doesn't seem to provide any way to test that the type or properties of the thrown object. I know we have test harnesses using assertion functions that already have some of these problems, but I don't see the benefit of adding a new set of methods with exactly the same issues. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
I'm not sure I grasp your overall point, but I have a few comments. On 03/06/14 11:22, Mike de Boer wrote: 1. The `Assert.*` namespace is optional and may be omitted. This module is also present in the addon-sdk and used _with_ that namespace, usually with a lowercase `assert.*`. Please pick whatever suits your fancy. FWIW I consider that like other code tests should be optimised for maintainability. Taking the position pick whatever doesn't help with this. 2. testharness.js, Mochitest, XPCShell’s head.js and other suite-runners that we use in-tree are needlessly monolithic. It's not needless if you want to run in non-Mozilla environments where jaascript modules don't yet exist. For platform-level tests, being able to cross check our implementation against other implementations has considerable value. They mix defining assertion methods, test setup, test definition, test teardown in one silo and dogmatically impose a test definition style. Their lack of modularity costs us flexibility in adopting and/ or promoting TDD development. They are, however, great for writing regression tests and that’s what we use them for. I'm not sure I understand this criticism. testharness.js and Mochitest, at least, are not really intended for writing unit tests, but for writing functional tests. I have had no problem using testharness.js in a setup where a comprehensive upfront testsuite was written in parallel with the code, which seems to be pretty close to a TDD ethos. I don't think that technical problems are preventing us adopting this development methodology. (Maybe for testing frontend code one can use mochitest for unit tests. I don't know how that works). 4. None of the test-suites promote modularity and needlessly dictate a reporting style. What I mean by this is that there’s no way to hook different reporting styles in a test runner to promote TDD, for example. What does automation use to detect test failures? TAP[1] is something efficient and optimised for machine-reading, but we parse output lines in a format that is far from an industry standard. We humans delve through a whole bunch of scroll back to find the test case/ assertion we’re interested in. We even rely on our tooling to repeat all the failing tests at the end of a run. Yes, the way we deal with test output has historically been suboptimal. We are in the process of fixing that as we speak. We have developed a json-based protocol [1] for test output. This is already being used for web-platform-tests, and for FirefoxOS certsuite. There is current work in progress for converting mochitest and marionette tests to this format. Other suites will follow. As you say, once we are using structured logging rather than trying to parse human-readable logging, it should be possible to do a lot more interesting things with the logging results. The structured logging package already comes with formatters for a small number of formats including, for example, something XUnit XML compatible. There are also lots of ideas for how to improve the user interface to test results in automation. These will come after the launch of treeherder. 5. Assertion semantics are indeed poorly specified, across the board. Our switch from `do_check_matches()` to `deepEqual()` even revealed a buggy implementation there, which we didn’t know about. Apart from that, it was largely undocumented, not fully covered by unit tests except for the pathological cases. I’m actually a bit scared of what I’ll find in Mochitest[3] Type coercion is something specifiable, but I’m not sure whether that is something `ok`, `equal` and family should implement guards for. If there’s a wish/ call for more specific assertion methods like `is_truthy`, `is_falsy` and variants for all possible coercion traps, I think there’s room in Assert.jsm to add those. We are in the sad status quo that all assertion methods in all test suites are underspecified to some degree. The fact that these methods are an integral part of each suite makes it harder to change that. Assert.jsm breaks away from that approach to make these improvements possible to a wider audience. If we agree that more spec’ing is needed, we might as well fork the spec[2] to MDN and collectively work it out. Changing the semantics of things that people are already using seems like a uphill battle. I think if you wanted to introduce a common set of assertion names across Mozilla harnesses, starting from CommonJS rather than starting from a discussion of actual requirements was the wrong approach. 7. Names of assertion methods are an excellent reason for bikeshedding. The main reason for the amount of time it took for the spec[2] to be formalised was exactly this, IIRC. Never mind that, like I said before: I’m fine with forking the spec and adding aliases for each assertion method if need be. I mostly care about the fact that we can implement them in one place. At least for testharness.js that would
Re: Standardized assertion methods
Please see my comments inline. On 03 Jun 2014, at 12:57, James Graham ja...@hoppipolla.co.uk wrote: I'm not sure I grasp your overall point, but I have a few comments. On 03/06/14 11:22, Mike de Boer wrote: 1. The `Assert.*` namespace is optional and may be omitted. This module is also present in the addon-sdk and used _with_ that namespace, usually with a lowercase `assert.*`. Please pick whatever suits your fancy. FWIW I consider that like other code tests should be optimised for maintainability. Taking the position pick whatever doesn't help with this. 2. testharness.js, Mochitest, XPCShell’s head.js and other suite-runners that we use in-tree are needlessly monolithic. It's not needless if you want to run in non-Mozilla environments where jaascript modules don't yet exist. For platform-level tests, being able to cross check our implementation against other implementations has considerable value. They mix defining assertion methods, test setup, test definition, test teardown in one silo and dogmatically impose a test definition style. Their lack of modularity costs us flexibility in adopting and/ or promoting TDD development. They are, however, great for writing regression tests and that’s what we use them for. I'm not sure I understand this criticism. testharness.js and Mochitest, at least, are not really intended for writing unit tests, but for writing functional tests. I have had no problem using testharness.js in a setup where a comprehensive upfront testsuite was written in parallel with the code, which seems to be pretty close to a TDD ethos. I don't think that technical problems are preventing us adopting this development methodology. (Maybe for testing frontend code one can use mochitest for unit tests. I don't know how that works). 4. None of the test-suites promote modularity and needlessly dictate a reporting style. What I mean by this is that there’s no way to hook different reporting styles in a test runner to promote TDD, for example. What does automation use to detect test failures? TAP[1] is something efficient and optimised for machine-reading, but we parse output lines in a format that is far from an industry standard. We humans delve through a whole bunch of scroll back to find the test case/ assertion we’re interested in. We even rely on our tooling to repeat all the failing tests at the end of a run. Yes, the way we deal with test output has historically been suboptimal. We are in the process of fixing that as we speak. We have developed a json-based protocol [1] for test output. This is already being used for web-platform-tests, and for FirefoxOS certsuite. There is current work in progress for converting mochitest and marionette tests to this format. Other suites will follow. Do you have bug numbers where I can follow that work in progress? It sounds awesome! As you say, once we are using structured logging rather than trying to parse human-readable logging, it should be possible to do a lot more interesting things with the logging results. The structured logging package already comes with formatters for a small number of formats including, for example, something XUnit XML compatible. There are also lots of ideas for how to improve the user interface to test results in automation. These will come after the launch of treeherder. 5. Assertion semantics are indeed poorly specified, across the board. Our switch from `do_check_matches()` to `deepEqual()` even revealed a buggy implementation there, which we didn’t know about. Apart from that, it was largely undocumented, not fully covered by unit tests except for the pathological cases. I’m actually a bit scared of what I’ll find in Mochitest[3] Type coercion is something specifiable, but I’m not sure whether that is something `ok`, `equal` and family should implement guards for. If there’s a wish/ call for more specific assertion methods like `is_truthy`, `is_falsy` and variants for all possible coercion traps, I think there’s room in Assert.jsm to add those. We are in the sad status quo that all assertion methods in all test suites are underspecified to some degree. The fact that these methods are an integral part of each suite makes it harder to change that. Assert.jsm breaks away from that approach to make these improvements possible to a wider audience. If we agree that more spec’ing is needed, we might as well fork the spec[2] to MDN and collectively work it out. Changing the semantics of things that people are already using seems like a uphill battle. I think if you wanted to introduce a common set of assertion names across Mozilla harnesses, starting from CommonJS rather than starting from a discussion of actual requirements was the wrong approach. That’s not what we’re doing here! Changing semantics is a non-goal, merging assertion methods into one re-usable module is. Taking the CommonJS spec as an umbrella for these
Re: Standardized assertion methods
On 03/06/2014 12:27, Mike de Boer wrote: snip 5. Assertion semantics are indeed poorly specified, across the board. Our switch from `do_check_matches()` to `deepEqual()` even revealed a buggy implementation there, which we didn’t know about. Apart from that, it was largely undocumented, not fully covered by unit tests except for the pathological cases. I’m actually a bit scared of what I’ll find in Mochitest[3] Type coercion is something specifiable, but I’m not sure whether that is something `ok`, `equal` and family should implement guards for. If there’s a wish/ call for more specific assertion methods like `is_truthy`, `is_falsy` and variants for all possible coercion traps, I think there’s room in Assert.jsm to add those. We are in the sad status quo that all assertion methods in all test suites are underspecified to some degree. The fact that these methods are an integral part of each suite makes it harder to change that. Assert.jsm breaks away from that approach to make these improvements possible to a wider audience. If we agree that more spec’ing is needed, we might as well fork the spec[2] to MDN and collectively work it out. Changing the semantics of things that people are already using seems like a uphill battle. I think if you wanted to introduce a common set of assertion names across Mozilla harnesses, starting from CommonJS rather than starting from a discussion of actual requirements was the wrong approach. That’s not what we’re doing here! Changing semantics is a non-goal, merging assertion methods into one re-usable module is. Taking the CommonJS spec as an umbrella for these simple assertion methods is a causality What are you trying to say here? I don't understand. I believe causality is not the word you want here. ~ Gijs ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 03/06/14 12:27, Mike de Boer wrote: 4. None of the test-suites promote modularity and needlessly dictate a reporting style. What I mean by this is that there’s no way to hook different reporting styles in a test runner to promote TDD, for example. What does automation use to detect test failures? TAP[1] is something efficient and optimised for machine-reading, but we parse output lines in a format that is far from an industry standard. We humans delve through a whole bunch of scroll back to find the test case/ assertion we’re interested in. We even rely on our tooling to repeat all the failing tests at the end of a run. Yes, the way we deal with test output has historically been suboptimal. We are in the process of fixing that as we speak. We have developed a json-based protocol [1] for test output. This is already being used for web-platform-tests, and for FirefoxOS certsuite. There is current work in progress for converting mochitest and marionette tests to this format. Other suites will follow. Do you have bug numbers where I can follow that work in progress? It sounds awesome! Yes, sorry I should have included some. The metabug for structured logging is bug 916295 The Mochitest work is happening in bug 886570 and Marionette in bug 956739. Treeherder is using pivotal tracker rather than bugzilla, so I don't have a bug number for that one, but the story for basic integration is [1]. Changing the semantics of things that people are already using seems like a uphill battle. I think if you wanted to introduce a common set of assertion names across Mozilla harnesses, starting from CommonJS rather than starting from a discussion of actual requirements was the wrong approach. That’s not what we’re doing here! Changing semantics is a non-goal, merging assertion methods into one re-usable module is. Taking the CommonJS spec as an umbrella for these simple assertion methods is a causality and I think it helps provide a common, immediate understanding for new contributors who’d like to write test for the code they contribute. Sure, the semantics of `do_check_matches()` changed. But that method was only used in two locations, its use not promoted in any wiki page and its implementation lossy. I was under the impression that you were proposing landing CommonJS method support and then forking the commonjs spec to improve the semantics. I may have misunderstood. I think I would need some evidence to back up the hypothesis that new contributers are unusually likely to know CommonJS compared to the N other test frameworks that exist. For example most automation contributers have a better working knowledge of Python than Javascript and would be more comfortable with something that looks like Python's unittest module. I imagine for front end developers there would be different biases. For platform developers the story is likely to be different again. In any case, learning new assertion names isn't something that strikes me as being a particularly high barrier to entry. With testharness.js the only complaint I recall about the names is that they favour explicitness over brevity. Confusion for people moving from other js test frameworks has more often come from the differences in high-level philosophy. [1] https://www.pivotaltracker.com/s/projects/749519/stories/70575430 ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
Indeed I meant to say the following: Taking the CommonJS spec as an umbrella for these simple assertion methods is more of a nice side-effect than it was the primary objective we started off with. I think it helps provide a common, immediate understanding for new contributors who’d like to write test for the code they contribute, as the add-on SDK and the NodeJS community already use it exclusively. On 03 Jun 2014, at 13:49, Gijs Kruitbosch gijskruitbo...@gmail.com wrote: On 03/06/2014 12:27, Mike de Boer wrote: snip 5. Assertion semantics are indeed poorly specified, across the board. Our switch from `do_check_matches()` to `deepEqual()` even revealed a buggy implementation there, which we didn’t know about. Apart from that, it was largely undocumented, not fully covered by unit tests except for the pathological cases. I’m actually a bit scared of what I’ll find in Mochitest[3] Type coercion is something specifiable, but I’m not sure whether that is something `ok`, `equal` and family should implement guards for. If there’s a wish/ call for more specific assertion methods like `is_truthy`, `is_falsy` and variants for all possible coercion traps, I think there’s room in Assert.jsm to add those. We are in the sad status quo that all assertion methods in all test suites are underspecified to some degree. The fact that these methods are an integral part of each suite makes it harder to change that. Assert.jsm breaks away from that approach to make these improvements possible to a wider audience. If we agree that more spec’ing is needed, we might as well fork the spec[2] to MDN and collectively work it out. Changing the semantics of things that people are already using seems like a uphill battle. I think if you wanted to introduce a common set of assertion names across Mozilla harnesses, starting from CommonJS rather than starting from a discussion of actual requirements was the wrong approach. That’s not what we’re doing here! Changing semantics is a non-goal, merging assertion methods into one re-usable module is. Taking the CommonJS spec as an umbrella for these simple assertion methods is a causality What are you trying to say here? I don't understand. I believe causality is not the word you want here. ~ Gijs ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 6/3/14, 8:39 AM, Mike de Boer wrote: I think it helps provide a common, immediate understanding for new contributors who’d like to write test for the code they contribute, as the add-on SDK and the NodeJS community already use it exclusively. I think there's a bit of functional area bias here, fwiw. I've seen very few (if any) Gecko contributors coming from either of those communities, for example. -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 6/3/14, 6:22 AM, Mike de Boer wrote: Their lack of modularity costs us flexibility in adopting and/ or promoting TDD development. Mike, I'm very curious about this part. Do you have a link offhand to a more detailed explanation of the issues here? Note that none of us think Mochitest is perfect by any means. And I think we all agree (or at least I agree!) that both the behavior an the names of the old xpcshell test functions is terrible. I do think we should be very intentional about adopting something new, both in terms of semantics (mochitest is() using == is a mistake we should not duplicate in the short-name comparison function in the new setup) and in terms of naming. That doesn't mean we shouldn't adopt it, but we should aim to fix the known problems when we do so, because it's unlikely that we'll get a second chance to change around the names to address them. -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 03 Jun 2014, at 14:54, Boris Zbarsky bzbar...@mit.edu wrote: On 6/3/14, 8:39 AM, Mike de Boer wrote: I think it helps provide a common, immediate understanding for new contributors who’d like to write test for the code they contribute, as the add-on SDK and the NodeJS community already use it exclusively. I think there's a bit of functional area bias here, fwiw. I've seen very few (if any) Gecko contributors coming from either of those communities, for example. Yes, I’m biased there I think. I hope that will change in the (near) future. Right now, the main area for NodeJS enthusiasts at Mozilla to go to is Services. following is slightly off-topic I want Firefox frontend development to be more mainstream, or at least be able to move faster and experiment more with JS development trends. (Note that Assert.jsm is not about a trend, it’s quite past that and several years old already). Indeed, I’m used to the NodeJS/ Mocha flow of writing tests as fast, or even faster, as writing the implementation of a feature. I could group tests, isolate one, hook in a debugger at any point and more. This is something I miss while working on Fx and to be honest: I still crave for more than a year later. Writing wrappers in python around things to improve the current situation like a band-aid isn’t the way I’m used to fix things; I like to take the bull by the horns[1] I’d like to ask _why_ structured logging needs to be bolted on top of what we have currently? Is it more work to fix the real problem? Are we less comfortable doing things in JS? We all want the same thing, I’m pretty sure of that. I’m just asking difficult questions to understand the decision of the direction we’re taking. Mike. [1] https://bugzilla.mozilla.org/show_bug.cgi?id=867742 -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
Re: Standardized assertion methods
On 03 Jun 2014, at 15:07, Boris Zbarsky bzbar...@mit.edu wrote: On 6/3/14, 8:50 AM, Boris Zbarsky wrote: I do think we should be very intentional about adopting something new, both in terms of semantics (mochitest is() using == is a mistake we should not duplicate in the short-name comparison function in the new setup) y One other note. The checkin so far preserved the assertion semantics in xpcshell, afaict: failure throws and terminates the test. Yes, deliberately so. I assume that the mochitest version will use a different reporter that doesn't throw-and-terminate, to preserve the current semantics of mochitest assertions. (If this assumption is incorrect, we need to have a separate discussion about that.) If so, we'll have the same methods but different semantics in the different harnesses. Not sure how much of a problem that is in practice... not least because I don't think people actually write xpcshell tests all that much. Ideally, run-until-failure would be a configurable option per test suite. For example, in automation we’d continue on failure and locally we’d stop on failure. Additionally, that’d be configurable per test and per run. Anyhow, Assert.jsm supports pluggable reporters, specifically meant to support any style. Everything will stay the way it currently is. -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
Re: Standardized assertion methods
On 03 Jun 2014, at 14:50, Boris Zbarsky bzbar...@mit.edu wrote: On 6/3/14, 6:22 AM, Mike de Boer wrote: Their lack of modularity costs us flexibility in adopting and/ or promoting TDD development. Mike, I'm very curious about this part. Do you have a link offhand to a more detailed explanation of the issues here? Nope, you got me there - I generalised too easily. This statement is based on personal experience, not science. Note that none of us think Mochitest is perfect by any means. And I think we all agree (or at least I agree!) that both the behavior an the names of the old xpcshell test functions is terrible. I do think we should be very intentional about adopting something new, both in terms of semantics (mochitest is() using == is a mistake we should not duplicate in the short-name comparison function in the new setup) and in terms of naming. That doesn't mean we shouldn't adopt it, but we should aim to fix the known problems when we do so, because it's unlikely that we'll get a second chance to change around the names to address them. I’d say that now that we’ve spliced it all out to a separate module, we’re more free to change things and make amendments. I don’t think it’s too late to change anything. I agree that `is()` paired with `==` was ill-advised, so now we have `equal()` and `strictEqual()` as well. To preserve backward compat, all the old function names of XPCShell assertions do still work. I was planning to do the same with Mochi. Mike. -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
Re: Standardized assertion methods
I understand all that and I *think* you missed the header mentioning I was going off-topic… Mike. On 03 Jun 2014, at 15:39, Gijs Kruitbosch gijskruitbo...@gmail.com wrote: On 03/06/2014 14:16, Mike de Boer wrote: Indeed, I’m used to the NodeJS/ Mocha flow of writing tests as fast, or even faster, as writing the implementation of a feature. I could group tests, isolate one, hook in a debugger at any point and more. This is something I miss while working on Fx and to be honest: I still crave for more than a year later. So I'm not used to the NodeJS/Mocha 'flow' of writing tests. Can you explain what the big difference there is? I find it hard to believe the names of the assertion functions are the one big thing making tests take much longer to write... Node is inherently single-threaded (unless you bolt on extra stuff), which makes the runtime environment a lot more controlled than say, a web browser. IME, the main issue in writing correct browser mochitest tests (which is what I write 95% of the time) is focus, timeouts, and event sequences (ensuring you wait for X before doing Y), OS differences (and sometimes, try/staging vs. my local machine differences) and making it easy and obvious (how) to trigger UI actions in a test, and the ease of debugging (which has gotten better now that we have --break-on-failure and --jsdebugger). That all seems to have preciously little to do with assertions, log output, or TDD per se. In other words, if we want to make the experience of writing tests easier, I don't think a unified assertion framework is where we should be focusing our effort. Can you expand on why you think that that is such an important component here, and how it compares with mocha/node? ~ Gijs ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
I don't think this is off-topic. I'm essentially asking: why did you focus on this, and why in this way? More broadly, I'm asking what it is you're missing from node/mocha. If you think that needs its own topic, feel free to fork the summary. In any case, discussions about the ease of use of our test framework(s) seem eminently on-topic for m.d.platform. ~ Gijs On 03/06/2014 14:45, Mike de Boer wrote: I understand all that and I *think* you missed the header mentioning I was going off-topic… Mike. On 03 Jun 2014, at 15:39, Gijs Kruitbosch gijskruitbo...@gmail.com wrote: On 03/06/2014 14:16, Mike de Boer wrote: Indeed, I’m used to the NodeJS/ Mocha flow of writing tests as fast, or even faster, as writing the implementation of a feature. I could group tests, isolate one, hook in a debugger at any point and more. This is something I miss while working on Fx and to be honest: I still crave for more than a year later. So I'm not used to the NodeJS/Mocha 'flow' of writing tests. Can you explain what the big difference there is? I find it hard to believe the names of the assertion functions are the one big thing making tests take much longer to write... Node is inherently single-threaded (unless you bolt on extra stuff), which makes the runtime environment a lot more controlled than say, a web browser. IME, the main issue in writing correct browser mochitest tests (which is what I write 95% of the time) is focus, timeouts, and event sequences (ensuring you wait for X before doing Y), OS differences (and sometimes, try/staging vs. my local machine differences) and making it easy and obvious (how) to trigger UI actions in a test, and the ease of debugging (which has gotten better now that we have --break-on-failure and --jsdebugger). That all seems to have preciously little to do with assertions, log output, or TDD per se. In other words, if we want to make the experience of writing tests easier, I don't think a unified assertion framework is where we should be focusing our effort. Can you expand on why you think that that is such an important component here, and how it compares with mocha/node? ~ Gijs ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
James, thanks so much for the additional background information about testing at Mozilla. I’m currently following the bugs you mentioned earlier and am looking forward to their results! Mike. On 03 Jun 2014, at 16:07, James Graham ja...@hoppipolla.co.uk wrote: On 03/06/14 14:16, Mike de Boer wrote: Writing wrappers in python around things to improve the current situation like a band-aid isn’t the way I’m used to fix things; I like to take the bull by the horns[1] I’d like to ask _why_ structured logging needs to be bolted on top of what we have currently? Is it more work to fix the real problem? Are we less comfortable doing things in JS? I'm not sure what wrappers in python you have in mind, but I think that there are a couple of important points to address here. The first is a general point about testing at Mozilla. There is a lot more going on than just testing of js code. We have tests written in, and for, C++, Javascript, Python, HTML, CSS and probably a bunch more things that I have forgotten. In terms of requirements it's pretty different from what's needed to test a small project entirely implemented in a single language. As it happens a lot of the orchestration of testing is implemented in Python. I doubt js was even a viable choice at the time this infrastructure was originally written, and now we have a set of mature libraries for dealing with lots of the incidental complexity of testing Mozilla products, like controlling system processes, and setting up B2G devices. This code is largely decoupled from the tests themselves and hopefully isn't something that most developers should need to care about. But the reason that we don't just throw it all away and start again is because doing so would be a huge cost for extremely uncertain benefit. That doesn't mean that we can't work to improve things where they are bad; I'm sure we all have areas that we think are ripe for change. In terms of structured logging in particular; I don't know why you think it's bolted on, or why it isn't fixing the real problem. To be honest I don't know what you think the real problem is. Structured logging is basically just an API for communicating the results of tests to anyone that cares to be notified of them, be it tbpl, mach, treeherder, an app running in your browser that collects test results, or whatever. The fact that it currently has a Python implementation just reflects the fact that a lot of our test infrastructure is Python, and doesn't mean that there's anything Python-specific about it. One could certainly implement StructuredLog.jsm that would be entirely interoperable with the Python code. Indeed, if you are writing a test harness that works entirely in javascript, I would strongly encourage you to do just that. The work on converting Mochitest to the new format might well have some code that you could reuse. If there are particular requirements that you think structured logging doesn't meet this is probably a good time to discuss them, since we haven't deployed it to too many places. Perhaps that discussion would be better off-list since it might not be of interest to everyone. ___ 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
Re: Standardized assertion methods
On 6/3/2014 8:39 AM, Gijs Kruitbosch wrote:
On 03/06/2014 14:16, Mike de Boer wrote:
Indeed, I’m used to the NodeJS/ Mocha flow of writing tests as fast,
or even faster, as writing the implementation of a feature. I could
group tests, isolate one, hook in a debugger at any point and more.
This is something I miss while working on Fx and to be honest: I
still crave for more than a year later.
So I'm not used to the NodeJS/Mocha 'flow' of writing tests. Can you
explain what the big difference there is? I find it hard to believe
the names of the assertion functions are the one big thing making
tests take much longer to write...
I'm used to xpcshell tests more than mochitests, and the biggest
difference by far between xpcshell and mocha that I'm aware of is that
mocha counts tests at finer granularity: xpcshell tests work on a
file-by-file basis, whereas mocha tests work at the level of individual
test('Name', function() {}) calls. With the right framework support,
this makes it much easier to debug and diagnose single failures when
you're testing a parser function, since you can enable and test only a
single instance instead of setting a breakpoint and continue'ing twelve
times to get to the one you want [fwiw, I have 822 tests in just 6 files
for one of my suites, although most of those are defined in giant array
comparisons].
--
Joshua Cranmer
Thunderbird and DXR developer
Source code archæologist
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On Tuesday 2014-06-03 15:21 +0200, Mike de Boer wrote: On 03 Jun 2014, at 15:07, Boris Zbarsky bzbar...@mit.edu wrote: I assume that the mochitest version will use a different reporter that doesn't throw-and-terminate, to preserve the current semantics of mochitest assertions. (If this assumption is incorrect, we need to have a separate discussion about that.) If so, we'll have the same methods but different semantics in the different harnesses. Not sure how much of a problem that is in practice... not least because I don't think people actually write xpcshell tests all that much. Ideally, run-until-failure would be a configurable option per test suite. For example, in automation we’d continue on failure and locally we’d stop on failure. Additionally, that’d be configurable per test and per run. Locally I generally want to see all the failures, since seeing the complete set of failures is often a much better hint as to the cause of the failures than just seeing the first one. -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: Standardized assertion methods
On 2014-06-02, 9:35 PM, Boris Zbarsky wrote: On 6/2/14, 5:33 PM, Gavin Sharp wrote: Do either of you have reasoning for that other than it looks better to me? My personal experience is that when I try to write xpcshell tests the amount of time it takes to type the test function names is very noticeable and actively interrupts my thinking about what I actually want to test... I find it much simpler to write mochitests than xpcshell tests for this reason. I'm quite willing to believe this is not the case for everyone else, of course. I personally think consistency trumps any personal preferences based on length/concision Of course given the existence of testharness we're not going to get consistency in mochitest anyway, even with this change. as long as what we end up with isn't unreasonably long/verbose. I think what xpcshell has now and what testharness says and what's being proposed (with the Assert. prefix) are unreasonably long/verbose. So, what is the decision about what was mentioned in the original post about reviewers requiring using Assert.jsm in mochitest-chrome/browser? I mostly care about the former, and I don't think anything in the subthread that got started with my reply addresses that point at all. Cheers, Ehsan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 03 Jun 2014, at 17:39, Ehsan Akhgari ehsan.akhg...@gmail.com wrote: On 2014-06-02, 9:35 PM, Boris Zbarsky wrote: On 6/2/14, 5:33 PM, Gavin Sharp wrote: Do either of you have reasoning for that other than it looks better to me? My personal experience is that when I try to write xpcshell tests the amount of time it takes to type the test function names is very noticeable and actively interrupts my thinking about what I actually want to test... I find it much simpler to write mochitests than xpcshell tests for this reason. I'm quite willing to believe this is not the case for everyone else, of course. I personally think consistency trumps any personal preferences based on length/concision Of course given the existence of testharness we're not going to get consistency in mochitest anyway, even with this change. as long as what we end up with isn't unreasonably long/verbose. I think what xpcshell has now and what testharness says and what's being proposed (with the Assert. prefix) are unreasonably long/verbose. So, what is the decision about what was mentioned in the original post about reviewers requiring using Assert.jsm in mochitest-chrome/browser? I mostly care about the former, and I don't think anything in the subthread that got started with my reply addresses that point at all. I was mainly pointing reviewers of (new) XPCShell tests towards Assert.jsm methods *without* prefix. I’ll remove them from the MDN docs as well. The namespace will remain optional. (Some tests already use it, which is perfectly fine). As for Mochitest-chrome and/ or browser: I’d like to defer that discussion and decision-making to bug 1018226 for those interested. Mike. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 6/3/14, 9:29 AM, Mike de Boer wrote: Nope, you got me there - I generalised too easily. This statement is based on personal experience, not science. I'm not looking for science, necessarily. I'm looking for an understanding of the problems we're trying to solve. My basic issue is that from my perspective what I'm seeing is a request that everyone change their workflow because the new setup is better without a clear description of the problems that the new setup is trying to solve. I'm sure there are problems that need solving here; I'm just trying to get a handle on what they are. To preserve backward compat, all the old function names of XPCShell assertions do still work. I was planning to do the same with Mochi. Sure. The big question is what we do moving forward for new tests. The way I see it, a test harness has the user-facing API (what test writers call) and it has a backend that tracks/reports the failures. Improvements to the backend are generally a no-brainer in my book. Changes to the user-facing API are something we should be more careful with... -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 6/3/14, 11:19 AM, L. David Baron wrote: Locally I generally want to see all the failures, since seeing the complete set of failures is often a much better hint as to the cause of the failures than just seeing the first one. Yes, exactly. -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 6/3/14, 11:17 AM, Joshua Cranmer wrote:
I'm used to xpcshell tests more than mochitests, and the biggest
difference by far between xpcshell and mocha that I'm aware of is that
mocha counts tests at finer granularity: xpcshell tests work on a
file-by-file basis, whereas mocha tests work at the level of individual
test('Name', function() {}) calls. With the right framework support,
this makes it much easier to debug and diagnose single failures
Indeed. I will assert that the current (now old?) xpcshell harness is
pretty terrible in terms of usability. It was one of the first
harnesses we added and we've learned a lot since then. Basically, my
take on the xpcshell tests is that pretty much any change would be an
improvement. Our other harnesses are not in that situation, though.
-Boris
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 2014-06-03, 1:49 PM, Gavin Sharp wrote: On Mon, Jun 2, 2014 at 9:35 PM, Boris Zbarsky bzbar...@mit.edu wrote: I think what xpcshell has now and what testharness says and what's being proposed (with the Assert. prefix) are unreasonably long/verbose. I suspected this is where we'd end up :) Reasonability is just as subjective as aesthetics. I really have a hard time accepting at face value the argument Assert.notEqual (or other shorter variants) is unreasonably long to type/paste repeatedly. Hacking on Gecko you have to frequently type much longer things :) I can certainly buy it's longer than what I'm used to, and even incremental effort is required - just not incremental effort is required and that effort is non-negligible given other factors :) Sure. But the point is, what does the proposed change buy us to make this subjective burden be worth it? That is what's not clear in this thread so far. At the lack of a good reason to change things, maintaining the status quo should be the default. ;) Cheers, Ehsan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 2014-06-03, 2:17 PM, Gavin Sharp wrote: I won't argue that a great case has been made :) But I see inherent value in consistency (both in the implementations and in the user-exposed API) for assertions across our in-tree test suites (or at least, across mochitest-based harnesses and xpcshell). Do you disagree? Not at all. I think what we disagree on is 1) how to address this inconsistency, and 2) what other things we should take into consideration while doing so. I think what's being proposed here so far is being more consistent with commonjs testing API (which to me seems like a clear non-goal) by just adding a new API which will only be used in new mochitest-chrome/browsers but not in the old ones (at least not yet) without addressing any of the problems with the SimpleTest API that we are aware of, or any of the issues raised with it (the subjectivity of the reasonability of the API should not be used to dismiss mine and Boris' concern on that). So it seems to me like doing what this proposal suggests is one step back in terms of being more consistent without any clear added value. Cheers, Ehsan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 6/3/14, 2:36 PM, Gregory Szorc wrote: There is a clear win in the ability to reuse, understand, and modify the common code. No one is arguing against having common harness code as far as I can see. I can't even recall which file(s) contain is/ok from mochitest SimpleTest.js. Which also contains everything else from mochitest. Assert.jsm makes logic centralized, consistent, and easy to extend. No one is arguing against that. I'm arguing against Assert.jsm using the commonjs API names. FWIW, a lot of this bikeshedding about what effectively boils down to style could be avoided if we had JavaScript code rewriting facilities that could atomically change all references. That wouldn't make writing new tests any easier, though it would of course ease the inconsistent styles during conversion issue. -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On Tue, Jun 3, 2014 at 12:31 PM, Boris Zbarsky bzbar...@mit.edu wrote: I can certainly buy it's longer than what I'm used to, and even incremental effort is required - just not incremental effort is required and that effort is non-negligible given other factors Purely subjectively, it's non-negligible. The more frustrating writing a test is, the less I want to do it, and the more I'll skimp on testing exhaustively... Maybe that's a personal failing of mine, but I suspect not. The way I see it, anything that makes the test-writing experience more frustrating is a bar to us having better test coverage. +1 to all of this. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 6/3/14, 3:31 PM, Boris Zbarsky wrote: Maybe that's a personal failing of mine, but I suspect not. More precisely, I expect it's a personal failing that is widespread, not just my personal little quirk. -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On Tue, Jun 3, 2014 at 12:41 PM, Bobby Holley bobbyhol...@gmail.com wrote: On Tue, Jun 3, 2014 at 12:31 PM, Boris Zbarsky bzbar...@mit.edu wrote: I can certainly buy it's longer than what I'm used to, and even incremental effort is required - just not incremental effort is required and that effort is non-negligible given other factors Purely subjectively, it's non-negligible. The more frustrating writing a test is, the less I want to do it, and the more I'll skimp on testing exhaustively... Maybe that's a personal failing of mine, but I suspect not. The way I see it, anything that makes the test-writing experience more frustrating is a bar to us having better test coverage. +1 to all of this. Agreed. I think that we should optimize our test harnesses for making it easy to write tests that contain a large number of useful assertions. SimpleTest.js does this pretty well. The amount of boiler plate that you need is really minimal. The new names from the initial email in this thread is certainly an improvement for xpcshell-test, but why not go all the way and align even more with SimpleTest.js? testharness.js still requires lots of boiler plate. Especially when writing async tests. And especially if you try to follow the rule that each test within a file should clean up after itself. I suspect this is part of the reason we haven't seen more people use it for normal regression-test writing. However SimpleTest.js certainly has shortcomings. * It's a pain when writing tests that trigger window.onerror. Generally we stick those tests in an iframe to not make SimpleTest.js think that we have an unhandled error. It'd be great if SimpleTest.js had a hook for indicating this error is expected * SimpleTest.js should use window.addEventListener(load) rather than window.onload. This would enable tests to use window.onload as it sees fit. * Given that promises are at this point the accepted way of doing asynchronous programming, it would be good to support something promise based instead of (or more likely in addition to) SimpleTest.waitForExplicitFinish(). * I think we overuse the third argument to is() and the second argument to ok(), and under-use info(). This causes people to type out more error logging code. This isn't so much a problem with SimpleTest.js as with how we use it though. * The fact that our httpd.js is completely non-standard and unlikely to ever run in other environments. Moving to node.js or pythons BaseHTTPServer seems much more likely to get it accepted in other environments. Node.js seems like the better choice given that all test writers will need to know JS anyway. Again, this isn't so much a problem with SimpleTest.js, but rather the environment that we use it in. / Jonas ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 03/06/14 22:28, Jonas Sicking wrote: testharness.js still requires lots of boiler plate. Especially when writing async tests. And especially if you try to follow the rule that each test within a file should clean up after itself. At this point testharness.js has taken several steps to allow more minimal tests. For example it has the concept of single page tests so that if you want to write one test per page you don't need to use lots of boilerplate; a test looks like titleTest the load event fires/title script src=/resources/testharness.js/script script src=/resources/testharnessreport.js/script scriptonload=done/script At this point I think the main high-level semantic difference with mochitest is that mochitest keeps going after asserts fail. I suspect this is part of the reason we haven't seen more people use it for normal regression-test writing. I suspect the stronger reason is that the process for getting testharness.js tests running on Mozilla infrastructure has been unclear. Until that is solved — something which I expect to happen soon — it is unreasonable to expect people to use it. * The fact that our httpd.js is completely non-standard and unlikely to ever run in other environments. Moving to node.js or pythons BaseHTTPServer seems much more likely to get it accepted in other environments. Node.js seems like the better choice given that all test writers will need to know JS anyway. Again, this isn't so much a problem with SimpleTest.js, but rather the environment that we use it in. At this point, more or less the whole of Mochitest, as used at Mozilla, is completely non-standard and unlikely to get accepted for running in other environments. web-platform-tests has standardised on a Python-based web server designed specifically for writing tests. Care has been taken to ensure that the test environment is portable to a number of different setups e.g. running on a central server, running on an individual's machine, running on automation, etc. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
Should we adopt these for SpiderMonkey's test suites, too? Porting tests between suites isn't something that's done frequently, but there are people writing tests for more than one suite, and being able to use the same assertion methods everywhere would be helpful for that. On Mon, Jun 2, 2014 at 12:37 PM, Mike de Boer mdeb...@mozilla.com wrote: Dear unit test writers, As a happy few of you might already know, we introduced a standalone, versatile class of assertion methods with Assert.jsm[1], which implements the CommonJS Unit Testing specification version 1.1[2]. These methods were already available to you in the global `Assert` namespace in XPCShell-test, Mochitest-browser and Mochitest-chrome tests. Since last Friday[3], each assertion method in Assert.jsm is available in the global scope of a unit test as well. Now we can say that the ‘old’ XPCShell-test assertion methods are deprecated in favour of the Assert.jsm ones. Here’s a short table of what this means in practice: * do_check_eq(a, b) — equal(a, b) * do_check_neq(a, b) — notEqual(a, b) * do_check_true(expr) — ok(expr) * do_check_false(expr) — ok(!expr) * do_check_null(expr) — equal(expr, null) * do_check_matches(a, b) — deepEqual(a, b) (undocumented XPCShell-test feature) Please note that we did NOT replace all occurrences with the Assert.jsm equivalents, we merely marked them as ‘deprecated’ in the docs[4]. We’re planning to do the same for Mochitest-browser tests in bug 1018226. Now, dear writer and reviewer of unit tests, I’d like to ask if you could take note of this change and apply it to new tests that’ll land in the tree. Finally, I’d like to thank Gregory Szorc, Ted Mielczarek, Blair McBride and many others who helped getting this change worked out, integrated and so on. But we’re not done yet! There are more things we can do to make reading, writing and tracing tests easier and more fun! Have fun, Mike. [1] https://developer.mozilla.org/en/docs/Mozilla/JavaScript_code_modules/Assert.jsm [2] http://wiki.commonjs.org/wiki/Unit_Testing/1.1 [3] https://bugzilla.mozilla.org/show_bug.cgi?id=1014482 [4] https://developer.mozilla.org/en-US/docs/Mozilla/QA/Writing_xpcshell-based_unit_tests [5] https://bugzilla.mozilla.org/show_bug.cgi?id=1018226 ___ 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
Re: Standardized assertion methods
Mike de Boer wrote: * do_check_eq(a, b) — equal(a, b) There's also strictEqual(a, b) for those like me who were wondering. -- Warning: May contain traces of nuts. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
Yes, that’d be very nice to have! In a perfect world, all the test suites/ runners we have end up using the same assertion methods. This would indeed greatly improve the portability of individual tests. Thanks for suggesting this, Till! On 02 Jun 2014, at 12:56, Till Schneidereit t...@tillschneidereit.net wrote: Should we adopt these for SpiderMonkey's test suites, too? Porting tests between suites isn't something that's done frequently, but there are people writing tests for more than one suite, and being able to use the same assertion methods everywhere would be helpful for that. On Mon, Jun 2, 2014 at 12:37 PM, Mike de Boer mdeb...@mozilla.com wrote: Dear unit test writers, As a happy few of you might already know, we introduced a standalone, versatile class of assertion methods with Assert.jsm[1], which implements the CommonJS Unit Testing specification version 1.1[2]. These methods were already available to you in the global `Assert` namespace in XPCShell-test, Mochitest-browser and Mochitest-chrome tests. Since last Friday[3], each assertion method in Assert.jsm is available in the global scope of a unit test as well. Now we can say that the ‘old’ XPCShell-test assertion methods are deprecated in favour of the Assert.jsm ones. Here’s a short table of what this means in practice: * do_check_eq(a, b) — equal(a, b) * do_check_neq(a, b) — notEqual(a, b) * do_check_true(expr) — ok(expr) * do_check_false(expr) — ok(!expr) * do_check_null(expr) — equal(expr, null) * do_check_matches(a, b) — deepEqual(a, b) (undocumented XPCShell-test feature) Please note that we did NOT replace all occurrences with the Assert.jsm equivalents, we merely marked them as ‘deprecated’ in the docs[4]. We’re planning to do the same for Mochitest-browser tests in bug 1018226. Now, dear writer and reviewer of unit tests, I’d like to ask if you could take note of this change and apply it to new tests that’ll land in the tree. Finally, I’d like to thank Gregory Szorc, Ted Mielczarek, Blair McBride and many others who helped getting this change worked out, integrated and so on. But we’re not done yet! There are more things we can do to make reading, writing and tracing tests easier and more fun! Have fun, Mike. [1] https://developer.mozilla.org/en/docs/Mozilla/JavaScript_code_modules/Assert.jsm [2] http://wiki.commonjs.org/wiki/Unit_Testing/1.1 [3] https://bugzilla.mozilla.org/show_bug.cgi?id=1014482 [4] https://developer.mozilla.org/en-US/docs/Mozilla/QA/Writing_xpcshell-based_unit_tests [5] https://bugzilla.mozilla.org/show_bug.cgi?id=1018226 ___ 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
Re: Standardized assertion methods
On 06/02/2014 01:49 PM, Mike de Boer wrote: Yes, that’d be very nice to have! In a perfect world, all the test suites/ runners we have end up using the same assertion methods. This would indeed greatly improve the portability of individual tests. As I said before (but was ignored), the more sensible approach to achieve that goal would be to adopt the testharness.js methods, which are also more consistently named, have better behaviour for the shorter-name methods, and are documented more clearly. HTH Ms2ger ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 6/2/2014 11:37 AM, Mike de Boer wrote: Since last Friday[3], each assertion method in Assert.jsm is available in the global scope of a unit test as well. Now we can say that the ‘old’ XPCShell-test assertion methods are deprecated in favour of the Assert.jsm ones. I think it's a very good thing that finally someone is doing the work of moving various different test suites towards unified assertion methods (regardless of the actual choice of which methods they are)! Here’s a short table of what this means in practice: * do_check_eq(a, b) — equal(a, b) * do_check_neq(a, b) — notEqual(a, b) * do_check_true(expr) — ok(expr) * do_check_false(expr) — ok(!expr) * do_check_null(expr) — equal(expr, null) * do_check_matches(a, b) — deepEqual(a, b) (undocumented XPCShell-test feature) Have you considered requiring test cases to use the the Assert. namespace explicitly? I would find that style more readable, and also assertions easier to find when scanning the code. And they're still shorter than current assertion functions in xpcshell tests. Especially the less-known Assert.throws(function, exception) seems to read better than just throws(function, exception). For example compare this snippet from the referenced bug... + equal(aPacket.type, paused); + let env = aPacket.frame.environment; + equal(env.type, function); + equal(env.function.name, banana3); + let parent = env.parent; + equal(parent.type, block); + ok(banana3 in parent.bindings.variables); + parent = parent.parent; + equal(parent.type, function); + equal(parent.function.name, banana2); + parent = parent.parent; + equal(parent.type, block); + ok(banana2 in parent.bindings.variables); + parent = parent.parent; + equal(parent.type, block); + ok(banana2 in parent.bindings.variables); + parent = parent.parent; + equal(parent.type, function); + equal(parent.function.name, banana); ...with this modified one: + Assert.equal(aPacket.type, paused); + let env = aPacket.frame.environment; + Assert.equal(env.type, function); + Assert.equal(env.function.name, banana3); + let parent = env.parent; + Assert.equal(parent.type, block); + Assert.ok(banana3 in parent.bindings.variables); + parent = parent.parent; + Assert.equal(parent.type, function); + Assert.equal(parent.function.name, banana2); + parent = parent.parent; + Assert.equal(parent.type, block); + Assert.ok(banana2 in parent.bindings.variables); + parent = parent.parent; + Assert.equal(parent.type, block); + Assert.ok(banana2 in parent.bindings.variables); + parent = parent.parent; + Assert.equal(parent.type, function); + Assert.equal(parent.function.name, banana); Much easier to visually separate what the test is doing from the intermixed calls that check the intermediate results. Cheers, Paolo ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
Warning: pet peeve coming up. I agree we should separate these things more clearly. I don't think making people type the same 7 characters repeatedly is a good way to do that. I think we should be more liberal with using blank lines instead. It's too often I see 30-line blocks of code with not a single line of whitespace separating logical blocks, leaving the (vegetarian or beef-greedy) reader the tedious tasks of separating out the meatballs from the metaphorical spaghetti code. Concretely, IMO in the code you cite there should be a blank line before each of the 'parent' reassignments. ~ Gijs On 02/06/2014 16:39, Paolo Amadini wrote: On 6/2/2014 11:37 AM, Mike de Boer wrote: Since last Friday[3], each assertion method in Assert.jsm is available in the global scope of a unit test as well. Now we can say that the ‘old’ XPCShell-test assertion methods are deprecated in favour of the Assert.jsm ones. I think it's a very good thing that finally someone is doing the work of moving various different test suites towards unified assertion methods (regardless of the actual choice of which methods they are)! Here’s a short table of what this means in practice: * do_check_eq(a, b) — equal(a, b) * do_check_neq(a, b) — notEqual(a, b) * do_check_true(expr) — ok(expr) * do_check_false(expr) — ok(!expr) * do_check_null(expr) — equal(expr, null) * do_check_matches(a, b) — deepEqual(a, b) (undocumented XPCShell-test feature) Have you considered requiring test cases to use the the Assert. namespace explicitly? I would find that style more readable, and also assertions easier to find when scanning the code. And they're still shorter than current assertion functions in xpcshell tests. Especially the less-known Assert.throws(function, exception) seems to read better than just throws(function, exception). For example compare this snippet from the referenced bug... + equal(aPacket.type, paused); + let env = aPacket.frame.environment; + equal(env.type, function); + equal(env.function.name, banana3); + let parent = env.parent; + equal(parent.type, block); + ok(banana3 in parent.bindings.variables); + parent = parent.parent; + equal(parent.type, function); + equal(parent.function.name, banana2); + parent = parent.parent; + equal(parent.type, block); + ok(banana2 in parent.bindings.variables); + parent = parent.parent; + equal(parent.type, block); + ok(banana2 in parent.bindings.variables); + parent = parent.parent; + equal(parent.type, function); + equal(parent.function.name, banana); with this modified one: + Assert.equal(aPacket.type, paused); + let env = aPacket.frame.environment; + Assert.equal(env.type, function); + Assert.equal(env.function.name, banana3); + let parent = env.parent; + Assert.equal(parent.type, block); + Assert.ok(banana3 in parent.bindings.variables); + parent = parent.parent; + Assert.equal(parent.type, function); + Assert.equal(parent.function.name, banana2); + parent = parent.parent; + Assert.equal(parent.type, block); + Assert.ok(banana2 in parent.bindings.variables); + parent = parent.parent; + Assert.equal(parent.type, block); + Assert.ok(banana2 in parent.bindings.variables); + parent = parent.parent; + Assert.equal(parent.type, function); + Assert.equal(parent.function.name, banana); Much easier to visually separate what the test is doing from the intermixed calls that check the intermediate results. Cheers, Paolo ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 02 Jun 2014, at 17:39, Paolo Amadini paolo.02@amadzone.org wrote: Have you considered requiring test cases to use the the Assert. namespace explicitly? I would find that style more readable, and also assertions easier to find when scanning the code. And they're still shorter than current assertion functions in xpcshell tests. I agree with you on that point, it makes the assertions easier to notice, so that’s why we’ve kept the `Assert.` namespace alive. In fact, I ‘forgot’ to remove the namespaced versions from https://developer.mozilla.org/en-US/docs/Mozilla/QA/Writing_xpcshell-based_unit_tests! It’s up to a matter of taste, I guess… Both are possible at the moment. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 6/2/2014 4:59 PM, Ehsan Akhgari wrote: I'm _pretty_ sure that the answer is no for mochitest-chrome at least. Are we running these tests out-of-tree in other environments? Paolo ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 6/2/2014 4:51 PM, Gijs Kruitbosch wrote: Concretely, IMO in the code you cite there should be a blank line before each of the 'parent' reassignments. I definitely agree, and I would also use the Assert. prefix to make the separation between action and check clearer (while if I understand correctly you'd prefer the shorter option). Cheers, Paolo ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On Mon, Jun 2, 2014 at 12:27 PM, Paolo Amadini paolo.02@amadzone.org wrote: On 6/2/2014 4:59 PM, Ehsan Akhgari wrote: I'm _pretty_ sure that the answer is no for mochitest-chrome at least. Are we running these tests out-of-tree in other environments? Do you mean by just opening the page in the browser? I don't think that's a common use case. But why do you ask? -- Ehsan http://ehsanakhgari.org/ ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 6/2/14, 3:34 PM, Paolo Amadini wrote: It seems to me that if we don't have external compatibility needs, we might as well move mochitests to use a set of assertion methods that is the same as xpcshell and maybe other test suites. Yes, but imho we should be moving xpcshell in the direction of the ok/is bits that mochitests use instead of moving mochitests in the direction of longer function names. -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On Mon, Jun 2, 2014 at 4:14 PM, Boris Zbarsky bzbar...@mit.edu wrote: On 6/2/14, 3:34 PM, Paolo Amadini wrote: It seems to me that if we don't have external compatibility needs, we might as well move mochitests to use a set of assertion methods that is the same as xpcshell and maybe other test suites. Yes, but imho we should be moving xpcshell in the direction of the ok/is bits that mochitests use instead of moving mochitests in the direction of longer function names. Yes, this was precisely my point. -- Ehsan http://ehsanakhgari.org/ ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
Do either of you have reasoning for that other than it looks better to me? I personally think consistency trumps any personal preferences based on length/concision, as long as what we end up with isn't unreasonably long/verbose. Gavin On Mon, Jun 2, 2014 at 4:47 PM, Ehsan Akhgari ehsan.akhg...@gmail.com wrote: On Mon, Jun 2, 2014 at 4:14 PM, Boris Zbarsky bzbar...@mit.edu wrote: On 6/2/14, 3:34 PM, Paolo Amadini wrote: It seems to me that if we don't have external compatibility needs, we might as well move mochitests to use a set of assertion methods that is the same as xpcshell and maybe other test suites. Yes, but imho we should be moving xpcshell in the direction of the ok/is bits that mochitests use instead of moving mochitests in the direction of longer function names. Yes, this was precisely my point. -- Ehsan http://ehsanakhgari.org/ ___ 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
Re: Standardized assertion methods
On 2014-06-02, 5:33 PM, Gavin Sharp wrote: Do either of you have reasoning for that other than it looks better to me? I personally think consistency trumps any personal preferences based on length/concision, as long as what we end up with isn't unreasonably long/verbose. I have two reasons: 1. I believe verbosity for test assertion functions is a bad thing. is/ok are easier to type/read than the Assert.jsm types. I assume that people familiar with CommonJS (which Assert.jsm seems to be based on) would perhaps be trained to ignore the verbosity of those functions, but people who write mochitest-chrome tests very frequently would probably not fall into that category. 2. I also value consistency more than my personal preferences, and based on that, using the existing APIs in some tests and the new APIs in other tests (even if we agreed that #1 above doesn't matter) is strictly worse than the status quo. Also, I'm not sure where the original discussion happened, and what the reasons are, but if the goal is to be more consistent, it seems like we need to move xpcshell tests to use the SimpleTest APIs since that seems to be the only test framework that we have which currently doesn't use it (ignoring testharness.js of course, and reftests don't really have an assertion API). I don't think adding a third (fourth considering th.js) and trying to transition mochitest-browser/chrome to it gradually gets us close to that goal. Cheers, Ehsan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 6/2/14, 3:42 PM, Ehsan Akhgari wrote: 2. I also value consistency more than my personal preferences, and based on that, using the existing APIs in some tests and the new APIs in other tests (even if we agreed that #1 above doesn't matter) is strictly worse than the status quo. btw, in the mozilla.dev.tech.javascript-engine.internals fork of this thread, bz and David Bruant pointed out that W3C's testharness and TC39's test262 each use yet another set of assertion function names. Any tests we import from those test suites will need glue code to integrate with our test harness(es). chris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 2014-06-02, at 16:24, Chris Peterson cpeter...@mozilla.com wrote: btw, in the mozilla.dev.tech.javascript-engine.internals fork of this thread, bz and David Bruant pointed out that W3C's testharness and TC39's test262 each use yet another set of assertion function names. Any tests we import from those test suites will need glue code to integrate with our test harness(es). Sounds like an area ripe for standardisation…I mean bike-shedding. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 6/2/14, 5:33 PM, Gavin Sharp wrote: Do either of you have reasoning for that other than it looks better to me? My personal experience is that when I try to write xpcshell tests the amount of time it takes to type the test function names is very noticeable and actively interrupts my thinking about what I actually want to test... I find it much simpler to write mochitests than xpcshell tests for this reason. I'm quite willing to believe this is not the case for everyone else, of course. I personally think consistency trumps any personal preferences based on length/concision Of course given the existence of testharness we're not going to get consistency in mochitest anyway, even with this change. as long as what we end up with isn't unreasonably long/verbose. I think what xpcshell has now and what testharness says and what's being proposed (with the Assert. prefix) are unreasonably long/verbose. -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 6/2/14, 7:26 PM, Martin Thomson wrote: Sounds like an area ripe for standardisation Note that standardizing several test suites on the same API might not work all that well when they have different goals and operating parameters, because what can you end up with is an API that doesn't quite fit any of them very well... -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Standardized assertion methods
On 6/2/14, 9:44 PM, Boris Zbarsky wrote: On 6/2/14, 7:26 PM, Martin Thomson wrote: Sounds like an area ripe for standardisation Note that standardizing several test suites on the same API might not work all that well when they have different goals and operating parameters Specifically, testharness and xpcshell are the ones I'm thinking about here. Mochitest is somewhere in between. The proposed API would work fine for mochitest and xpcshell, but not for testharness. -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
