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 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. > >> 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 directly conflict with the hard > requirement for cross-browser compatibility. The specific choice of > methods here would also conflict with other lower level goals like > consistency of style and ease of writing good tests. > > [1] http://mozbase.readthedocs.org/en/latest/mozlog_structured.html > _______________________________________________ > 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
