Add check for failed binding comparison #1176
Conversation
| it('udpates bound components when the value changes', function(done) { | ||
| expect(gd.layout.sliders[0].active).toBe(0); | ||
|
|
||
| Plotly.restyle(gd, 'marker.color', 'ecru').then(function() { |
There was a problem hiding this comment.
what's the different between this test case and the one above?
There was a problem hiding this comment.
... apart from 'ecru' not being a valid value.
There was a problem hiding this comment.
Ah, of course. My mistake. It's an end-to-end test, not a unit test. I added black which is valid but not present. I also modified the test name to read does not update the component if the value is not present.
There was a problem hiding this comment.
(this is the danger of development-driven tests vs. test-driven development.)
| @@ -462,6 +462,52 @@ describe('update menus interactions', function() { | |||
| }); | |||
| }); | |||
|
|
|||
| it('should update correctly on failed binding comparisons', function(done) { | |||
There was a problem hiding this comment.
might as well, right @rreusser ?
There was a problem hiding this comment.
Ah, a full end to end test of the specific failing case. Thanks! 👍
To elaborate, the specific difficulty is knowing when to dive into array contents. It's a little unpleasant since… do you do a deep comparison? Limit the deep comparison to when there aren't 1000 elements?
To limit the intelligence (which correlates with bugginess), my solution was to only compare arrays if they're length 1. I think that's a reasonable compromise since overall there are quite a few tests here (though this case was missed, obv 😞 )
A simple fix for #1169. The fix is more straightforward than the test, which I'm adding now.