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
