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
