Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
KatieWoe opened this issue Feb 5, 2021 · 10 comments
Closed

Comments

@KatieWoe
Copy link
Contributor

KatieWoe commented Feb 5, 2021

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
Copy link
Contributor

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
Copy link
Contributor

jbphet commented Mar 11, 2021

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

  • load the sim
  • go to the first screen, and select the second scene (the one with the piggy banks)
  • turn on the 2nd piggy bank by flipping the AB switch at the right of the screen
  • start dragging the 2nd piggy bank (the one above the number line)
  • while keeping a finger on the 2nd piggy bank, turn off its visibility using the AB switch
  • move the finger that was on the 2nd piggy bank (which is now invisible)

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 added a commit to phetsims/number-line-common that referenced this issue Mar 12, 2021
@jbphet
Copy link
Contributor

jbphet commented Mar 12, 2021

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
Copy link
Contributor

jbphet commented Mar 12, 2021

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
Copy link
Contributor

jbphet commented Mar 12, 2021

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 jbphet closed this as completed Mar 12, 2021
@jbphet
Copy link
Contributor

jbphet commented Apr 5, 2021

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

@jbphet jbphet reopened this Apr 5, 2021
@KatieWoe
Copy link
Contributor Author

KatieWoe commented Apr 5, 2021

I ran a fuzz test on the debug of phetsims/qa#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
Copy link
Contributor

jbphet commented Apr 8, 2021

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

  • load the sim
  • go to the first screen and stay one the first scene (elevation)
  • drag one of the items (the girl, bird, or fish) into the scene and drop it
  • with one finger, drag the spherical point controller that is attached to the number line up and down a little
  • without releasing the spherical point controller, use another finger and drag the item in the elevation scene out of the scene
  • move the finger that was dragging the spherical point controller up and down

Here's a screen capture of the sequence:

nli-point-controller-assert

@jbphet
Copy link
Contributor

jbphet commented Apr 9, 2021

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
Copy link
Contributor Author

Looks good in rc.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants