Changing the style guide's preference for loose over strict equality checks in non-test code (was: Re: PSA: The mochitest ise() function is dead, please use is() instead)
On 14/05/2015 01:21, Martin Thomson wrote: On Wed, May 13, 2015 at 4:54 PM, Matthew N. mattn+firefox-...@mozilla.com wrote: In JavaScript, == is preferred to ===. from https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style#Operators Ahh, that's where it was hiding. Can we reverse that statement please? Concurring with Mike in the other branch of the original thread, I would prefer not to (for non-test code). You've also provided no arguments as to why we should be doing this. ~ Gijs ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: PSA: The mochitest ise() function is dead, please use is() instead
On 14 May 2015, at 03:15, Boris Zbarsky bzbar...@mit.edu wrote: On 5/13/15 7:35 PM, Gregory Szorc wrote: I would steer people in the direction of Assert.jsm, specifically Assert.deepEqual This should be used very very carefully. As a very simple example, using this (or worse yet notDeepEqual) in any test that tries to check for equality of Window objects is not a good idea. The idea is *I think* that ObjectUtils.jsm (which Assert.jsm uses since recently) can be extended to be aware of various object types. Right now it doesn’t have optimal support for iterables like `Map` and `Set`. Same goes for Gecko wrapper objects, like `window`. When you want to use `deepEqual` with those, anyone should feel free to add support for it! Even if it’s only rudimentary support only. Regarding the `==` vs. `===` discussion: I think using `===` for in-test assertions is obviously preferred. This is where Assert.jsm can help and I’d be happy with a patch that replaces `Assert.equal` with `Assert.strictEqual` and just drop the `Assert.strict*` family of methods. In general I think it’s up to a dev’s own discretion which to use; it’s never been a footgun for me because I know each of their semantics inside-out[1]. TBH, I’ve never seen any of my colleagues encounter type coercion bugs over the past eight years. Maybe I’m just lucky, but I do know that the danger of `==` was hyped a bit when it first hit the blogosphere, followed by the (in)famous wtfjs.com site, a couple of years ago and has been a recurring theme ever since. I hope this helps, Mike. [1] No, I’m not trying to pat myself on the back, sometimes this is just the case when you’re a professional ;-) ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: PSA: The mochitest ise() function is dead, please use is() instead
On 14/05/15 00:35, Gregory Szorc wrote: I would steer people in the direction of Assert.jsm, specifically Assert.deepEqual, which uses ObjectUtils.jsm goodness for type aware comparisons so things like Date, RegExp, and Object comparisons have sane behavior. (deepEqual falls back to === for non-special types. If you are writing tests for web-exposed APIs you should strongly consider writing web-platform-tests instead of mochitests. These provide an assert_equals function for equality testing which does what the ES spec calls SameValue equality; like === for most cases but with -0 != 0 and NaN === NaN. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: PSA: The mochitest ise() function is dead, please use is() instead
On 5/14/15 8:03 AM, Mike de Boer wrote: Same goes for Gecko wrapper objects, like `window`. When you want to use `deepEqual` with those, anyone should feel free to add support for it! Here's the thing. When comparing window objects, in what sense would anything other than === be useful? And in general, deepEquals (structural type equality) and === (reference identity equality) are just completely different operations for objects. Sometimes you want one, sometimes the other. In practice, for tests of the DOM, structural type equality is almost never the desired type of equality. -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: PSA: The mochitest ise() function is dead, please use is() instead
On 2015-05-14 8:03 AM, Mike de Boer wrote: On 14 May 2015, at 03:15, Boris Zbarsky bzbar...@mit.edu wrote: On 5/13/15 7:35 PM, Gregory Szorc wrote: I would steer people in the direction of Assert.jsm, specifically Assert.deepEqual This should be used very very carefully. As a very simple example, using this (or worse yet notDeepEqual) in any test that tries to check for equality of Window objects is not a good idea. The idea is *I think* that ObjectUtils.jsm (which Assert.jsm uses since recently) can be extended to be aware of various object types. Right now it doesn’t have optimal support for iterables like `Map` and `Set`. Same goes for Gecko wrapper objects, like `window`. When you want to use `deepEqual` with those, anyone should feel free to add support for it! Even if it’s only rudimentary support only. But the point is that there is no use to deep comparing something like two window objects. If you are doing that, you're almost definitely doing something wrong. This is why usage of these Assert.jsm functions should be highly discouraged at least in tests examining the Web platform. The semantics are too hard to understand, and they may not be what we want (as is the case with the deep equality checks) at all. (FWIW, I noticed recently that someone had edited the Mochitest MDN page claiming that the usage of Assert.jsm is encouraged. I have updated that page to match the reality.) ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Changing the style guide's preference for loose over strict equality checks in non-test code (was: Re: PSA: The mochitest ise() function is dead, please use is() instead)
On Thu, May 14, 2015 at 9:14 AM, Martin Thomson m...@mozilla.com wrote: On Thu, May 14, 2015 at 5:17 AM, Gijs Kruitbosch gijskruitbo...@gmail.com wrote: On 14/05/2015 01:21, Martin Thomson wrote: On Wed, May 13, 2015 at 4:54 PM, Matthew N. mattn+firefox-...@mozilla.com wrote: In JavaScript, == is preferred to ===. from https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style#Operators Ahh, that's where it was hiding. Can we reverse that statement please? Concurring with Mike in the other branch of the original thread, I would prefer not to (for non-test code). You've also provided no arguments as to why we should be doing this. I thought that this was understood. Crockford offers plenty of reasons in his book. To summarize, == uses implicit type coercion. It says that you don't care or that you don't know what something is. I've never seen case where an explicit check is not possible, and does not help make code clearer. FWIW, I concur with Martin for the reasons he states. I don't think anyone is saying that we should change existing code, but this would be clearer for new code. -Ekr ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Changing the style guide's preference for loose over strict equality checks in non-test code (was: Re: PSA: The mochitest ise() function is dead, please use is() instead)
On Thu, May 14, 2015 at 5:17 AM, Gijs Kruitbosch gijskruitbo...@gmail.com wrote: On 14/05/2015 01:21, Martin Thomson wrote: On Wed, May 13, 2015 at 4:54 PM, Matthew N. mattn+firefox-...@mozilla.com wrote: In JavaScript, == is preferred to ===. from https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style#Operators Ahh, that's where it was hiding. Can we reverse that statement please? Concurring with Mike in the other branch of the original thread, I would prefer not to (for non-test code). You've also provided no arguments as to why we should be doing this. I thought that this was understood. Crockford offers plenty of reasons in his book. To summarize, == uses implicit type coercion. It says that you don't care or that you don't know what something is. I've never seen case where an explicit check is not possible, and does not help make code clearer. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: PSA: The mochitest ise() function is dead, please use is() instead
On Wed, May 13, 2015 at 3:52 PM, Martin Thomson m...@mozilla.com wrote: On Wed, May 13, 2015 at 3:46 PM, Ehsan Akhgari ehsan.akhg...@gmail.com wrote: ise() was an alias to is() as of bug 949614. I landed bug 1154275 on inbound today which removes ise() and replaces its usages with is(). Since is() now does a === comparison by default, there should be no reason to use ise(). \o/ Incidentally, are we prepared to have a discussion about the relative merits of == and ===? I see a lot of code that is open to type coercion bugs and I've been told that == is in the official style (though I note its absence from https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style#JavaScript_practices just now). Apologies if I hijack your thread. I do want to continue to strongly encourage wider use of code deletion as a means of making progress. I consider use of == in JavaScript testing code to be a bug: the == operator is just too wonky. I would steer people in the direction of Assert.jsm, specifically Assert.deepEqual, which uses ObjectUtils.jsm goodness for type aware comparisons so things like Date, RegExp, and Object comparisons have sane behavior. (deepEqual falls back to === for non-special types. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: PSA: The mochitest ise() function is dead, please use is() instead
On Wed, May 13, 2015 at 3:46 PM, Ehsan Akhgari ehsan.akhg...@gmail.com wrote: ise() was an alias to is() as of bug 949614. I landed bug 1154275 on inbound today which removes ise() and replaces its usages with is(). Since is() now does a === comparison by default, there should be no reason to use ise(). \o/ Incidentally, are we prepared to have a discussion about the relative merits of == and ===? I see a lot of code that is open to type coercion bugs and I've been told that == is in the official style (though I note its absence from https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style#JavaScript_practices just now). Apologies if I hijack your thread. I do want to continue to strongly encourage wider use of code deletion as a means of making progress. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: PSA: The mochitest ise() function is dead, please use is() instead
On Wed, May 13, 2015 at 4:54 PM, Matthew N. mattn+firefox-...@mozilla.com wrote: In JavaScript, == is preferred to ===. from https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style#Operators Ahh, that's where it was hiding. Can we reverse that statement please? ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: PSA: The mochitest ise() function is dead, please use is() instead
On 5/13/15 7:35 PM, Gregory Szorc wrote: I would steer people in the direction of Assert.jsm, specifically Assert.deepEqual This should be used very very carefully. As a very simple example, using this (or worse yet notDeepEqual) in any test that tries to check for equality of Window objects is not a good idea. -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
