https://codereview.chromium.org/22962009/diff/5001/test/cctest/test-object-observe.cc
File test/cctest/test-object-observe.cc (right):

https://codereview.chromium.org/22962009/diff/5001/test/cctest/test-object-observe.cc#newcode479
test/cctest/test-object-observe.cc:479: TEST(ObserveNamedAccessCheck) {
On 2013/08/16 13:51:54, rossberg wrote:
Would be good to refine the test such that only individual properties
are
blocked, while others pass (for both named and indexed). E.g. by
replacing your
blockAccess mechanics with a black list.

Done.

https://codereview.chromium.org/22962009/diff/5001/test/cctest/test-object-observe.cc#newcode488
test/cctest/test-object-observe.cc:488: CompileRun("records = null;"
On 2013/08/16 13:51:54, rossberg wrote:
Nit: can we make these proper 'var' declarations?

Done.

https://codereview.chromium.org/22962009/diff/5001/test/cctest/test-object-observe.cc#newcode499
test/cctest/test-object-observe.cc:499: CompileRun("records2 = null;"
On 2013/08/16 13:51:54, rossberg wrote:
Same here.

Done.

https://codereview.chromium.org/22962009/diff/5001/test/cctest/test-object-observe.cc#newcode503
test/cctest/test-object-observe.cc:503: "obj.foo = 'bar';"
On 2013/08/16 13:51:54, rossberg wrote:
Please add other forms of mutation, in particular, via
Object.defineProperty and
via the API.

Happy to expand this a little bit, but I'd be interested in what you
think counts as good coverage; see my question below about complex array
methods.

https://codereview.chromium.org/22962009/diff/5001/test/cctest/test-object-observe.cc#newcode585
test/cctest/test-object-observe.cc:585: "[].pop.call(obj);"
On 2013/08/16 13:51:54, rossberg wrote:
Can we check some more complex array functions, too?

How complex do you want? It's not clear to me what stuff should be
tested here, vs just testing access checks in general and testing record
enqueueing in general.

We have tests in mjsunit for testing that we get the right records in
Object.observe; as long as there's a reasonably tight bottleneck in
src/object-observe.js for where we enqueue records, I wouldn't expect
there to be any increase in test coverage from testing, say, 2 array
methods compared to 5 array methods.

I ask because writing tests in this style (as API tests) is
super-verbose and time consuming, compared to writing tests in JS, so if
I can get more coverage out of the JS tests I'd much rather do that.

https://codereview.chromium.org/22962009/diff/5001/test/cctest/test-object-observe.cc#newcode587
test/cctest/test-object-observe.cc:587: // TODO(adamk): Support splice
records
On 2013/08/16 13:51:54, rossberg wrote:
What is this TODO about?

Made it longer.

https://codereview.chromium.org/22962009/

--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Reply via email to