phetsims / number-line-integers

"Number Line: Integers" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
0 stars 4 forks source link

CT should not be in this situation if controller is always locked to a point #106

Closed KatieWoe closed 3 years ago

KatieWoe commented 3 years ago
number-line-integers : multitouch-fuzz : unbuilt
https://bayes.colorado.edu/continuous-testing/ct-snapshots/1612502607679/number-line-integers/number-line-integers_en.html?continuousTest=%7B%22test%22%3A%5B%22number-line-integers%22%2C%22multitouch-fuzz%22%2C%22unbuilt%22%5D%2C%22snapshotName%22%3A%22snapshot-1612502607679%22%2C%22timestamp%22%3A1612533255989%7D&brand=phet&ea&fuzz&fuzzPointers=2&memoryLimit=1000
Query: brand=phet&ea&fuzz&fuzzPointers=2&memoryLimit=1000
Uncaught Error: Assertion failed: should not be in this situation if controller is always locked to a point
Error: Assertion failed: should not be in this situation if controller is always locked to a point
at window.assertions.assertFunction (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1612502607679/assert/js/assert.js:25:13)
at PointController.proposePosition (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1612502607679/number-line-common/js/common/model/PointController.js:297:17)
at DragListener.drag [as _dragListener] (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1612502607679/number-line-common/js/common/view/PointControllerNode.js:190:27)
at DragListener.drag (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1612502607679/scenery/js/listeners/PressListener.js:446:10)
at DragListener._dragAction.Action.parameters.name (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1612502607679/scenery/js/listeners/DragListener.js:244:36)
at Action.execute (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1612502607679/axon/js/Action.js:225:18)
at DragListener.drag (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1612502607679/scenery/js/listeners/DragListener.js:363:22)
at DragListener.pointerMove (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1612502607679/scenery/js/listeners/PressListener.js:809:14)
at Input.dispatchToListeners (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1612502607679/scenery/js/input/Input.js:1832:25)
at Input.dispatchEvent (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1612502607679/scenery/js/input/Input.js:1781:10)
id: Bayes Chrome
Snapshot from 2/4/2021, 10:23:27 PM
pixelzoom commented 3 years ago

In 2/4/21 dev meeting, we established that this may be due to CT multi-touch fuzz testing ( ?fuzzPointers=2) being enabled for phetsims/aqua#106.

jbphet commented 3 years ago

I've done some investigation on this, and I can see what's happening. Here is how to duplicate the problem manually.

Because the pointer associated with the 2nd piggy bank is still around and able to send messages to the drag listener, it does, which in turn tries to move the piggy bank, which then tries to move a point on the number line, but that point no longer exists, so an assertion is hit. I think if we interrupt input on the point controller in this case, the problem should go away.

jbphet commented 3 years ago

I've added code to cancel interactions with the point controller when it becomes invisible. After this change the problem can no longer be duplicated with the sequence described above. I'll fuzz test locally and keep an eye on CT and, if I don't see a recurrence, I'll close.

jbphet commented 3 years ago

After this change, I ran fuzz testing locally using the query params in the issue report above (brand=phet&ea&fuzz&fuzzPointers=2&memoryLimit=1000) for an hour with no problems. The only caveat is that my screen saver seems to have kicked in, which may have stopped the browser from running, but at any rate, it ran for a while.

jbphet commented 3 years ago

I ran it for a while longer, so local fuzz testing was at least an hour. CT is clear after about 17 hours. I'm pretty confident in this fix, and the testing so far looks good, so I'm going to go ahead and close this one.

jbphet commented 3 years ago

This is still popping up on CT, so I'm re-opening. Here's a stack trace from the most recent occurrence:

number-line-integers : multitouch-fuzz : unbuilt
https://bayes.colorado.edu/continuous-testing/ct-snapshots/1617648211377/number-line-integers/number-line-integers_en.html?continuousTest=%7B%22test%22%3A%5B%22number-line-integers%22%2C%22multitouch-fuzz%22%2C%22unbuilt%22%5D%2C%22snapshotName%22%3A%22snapshot-1617648211377%22%2C%22timestamp%22%3A1617651087188%7D&brand=phet&ea&fuzz&fuzzPointers=2&memoryLimit=1000&supportsPanAndZoom=false
Query: brand=phet&ea&fuzz&fuzzPointers=2&memoryLimit=1000&supportsPanAndZoom=false
Uncaught Error: Assertion failed: should not be in this situation if controller is always locked to a point
Error: Assertion failed: should not be in this situation if controller is always locked to a point
at window.assertions.assertFunction (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1617648211377/assert/js/assert.js:25:13)
at PointController.proposePosition (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1617648211377/number-line-common/js/common/model/PointController.js:297:17)
at DragListener.drag [as _dragListener] (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1617648211377/number-line-common/js/common/view/PointControllerNode.js:193:27)
at DragListener.drag (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1617648211377/scenery/js/listeners/PressListener.js:450:10)
at DragListener._dragAction.Action.parameters.name (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1617648211377/scenery/js/listeners/DragListener.js:243:36)
at Action.execute (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1617648211377/axon/js/Action.js:225:18)
at DragListener.drag (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1617648211377/scenery/js/listeners/DragListener.js:362:22)
at DragListener.pointerMove (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1617648211377/scenery/js/listeners/PressListener.js:815:14)
at Input.dispatchToListeners (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1617648211377/scenery/js/input/Input.js:1879:25)
at Input.dispatchEvent (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1617648211377/scenery/js/input/Input.js:1828:10)
id: Bayes Chrome
Snapshot from 4/5/2021, 12:43:31 PM
KatieWoe commented 3 years ago

I ran a fuzz test on the debug of https://github.com/phetsims/QA/issues/634 with the above parameters and did see this error once. It didn't freeze the sim and I didn't catch it visually, and I haven't reproduced it manually, so I'm still not sure of the cause, but it does seem to be in rc.

jbphet commented 3 years ago

I've tracked this down, here's how to reproduce it manually. This is distinct from the sequence reported above.

Here's a screen capture of the sequence:

nli-point-controller-assert

jbphet commented 3 years ago

Both of the scenarios in which this problem could occur have now been fixed on both master and in the 1.1 branch. I'm about to roll a new RC, and will have QA verify this fix on that version.

KatieWoe commented 3 years ago

Looks good in rc.2