test: Tests for issue #20 #21
Conversation
ravijayaramappa
force-pushed
the
ravi/master/byPassMembrane
branch
2 times, most recently
from
5c0a1c8 to
b7b93f8
Compare
ravijayaramappa
force-pushed
the
ravi/master/byPassMembrane
branch
from
b7b93f8 to
6e96cfc
Compare
| @@ -43,7 +43,7 @@ function lockShadowTarget(membrane: ReactiveMembrane, shadowTarget: ReactiveMemb | |||
| targetKeys.forEach((key: PropertyKey) => { | |||
| let descriptor = getOwnPropertyDescriptor(originalTarget, key) as PropertyDescriptor; | |||
|
|
|||
| // We do not need to wrap the descriptor if not configurable | |||
| // We do not need to wrap the descriptor if configurable | |||
There was a problem hiding this comment.
Line 51 does the opposite of the original comment.
Test to demostrate that membrane containment can be bypassed using getOwnPropertyDescriptor()
ravijayaramappa
force-pushed
the
ravi/master/byPassMembrane
branch
from
6e96cfc to
bcd1d28
Compare
| const { get } = desc; | ||
| expect(() => { | ||
| get().foo = ''; | ||
| }).toThrow(); |
There was a problem hiding this comment.
perhaps this should become a test of its own?
There was a problem hiding this comment.
The second assertion(L292) is verifying that the data wasn't mutated. To separate it to its own test case I would have to copy the whole thing, place the mutation attempt(L290) in a try-catch. Seems too much duplication for asserting that read only proxy is blocking the mutation.
And I have the test case description wrong, will fix that.
test/read-only-handler.spec.ts
Outdated
| }).toThrow(); | ||
| expect(todos['entry'].foo).toEqual('bar'); | ||
| }); | ||
| }) |
There was a problem hiding this comment.
aside from assertion to throw, what is the difference between the tests in this file and reactive-handler.spec.ts? Wondering if we are duplicating here
There was a problem hiding this comment.
They are testing two different APIs. Test cases in reactive-handler.spec.ts is testing that the inner object is wrapped in a reactive proxy. The test cases in this file is testing that the readonly proxy cannot be bypassed.
The testcase name is wrong. Will fix it.
There was a problem hiding this comment.
Both the tests exploit the same loop hole in the implementation.
ravijayaramappa
force-pushed
the
ravi/master/byPassMembrane
branch
from
30e5206 to
67c16cd
Compare
|
Looking good! We will fix this soon. |
Tests for issue #20