#3180. Add JsAnyOperation.and tests #3216
Conversation
There was a problem hiding this comment.
LGTM.
Just wondering about the semantics of repeated executions of eval('var v = e; ...'): Is it safe to assume that nothing changes observably when it is executed for the second, third, etc time?
@srujzs, perhaps you could suggest another interop expert reviewer, just so we don't put too much of a burden on you? Otherwise, it would be great if you could take a look!
I'd avoid eval with At a meta level, I wonder if these tests are a bit overkill. There are tens of these operators, the implementation is quite direct, and there are a lot of values that can be used with these operators (any JS value, in fact!). Maybe it makes sense to test them all as thoroughly as we are, but I'm worried we'll be spending too many test cycles on something that will break in an obvious way if it ever changes. Thoughts, @eernstg?
I'm happy to keep taking a look at all the PRs! @nshahan has expressed interest in taking a look as well. |
| Expect.equals(globalContext["one"], JSObject().and(1.toJS)); | ||
| Expect.equals(globalContext["string"], JSObject().and("s".toJS)); | ||
| Expect.listEquals( | ||
| globalContext["a1"].dartify(), |
There was a problem hiding this comment.
Like the comments on the / PR, there's no reason to do dartify().
| Expect.equals(globalContext["string"], JSObject().and("s".toJS)); | ||
| Expect.listEquals( | ||
| globalContext["a1"].dartify(), | ||
| JSObject().and([].jsify()).dartify(), |
There was a problem hiding this comment.
Also like the comments on the / PR, prefer fetching the array/object from the global context here, on line 41, and on line 55. Same comments everywhere else.
| eval(r''' | ||
| function foo() {} | ||
|
|
||
| var object = {}; |
There was a problem hiding this comment.
Same meta-level comment as the / PR - instead of replicating this test file we can just use eval to have a different definition for the LHS JS value.
|
Replaced by #3223 |
No description provided.