phetsims/aqua

Enable multitouch testing on CT

Closed this issue · 18 comments

jessegreenberg commented

We would like to enable multitouch fuzzing on CT testing. It would be good to cover multitouch testing on CT in general, but this will also cover magnification testing in CT. First step is to fuzz everything with ?fuzzPointers=3.

First step is to test all sims with multitouch fuzzing and get a sense of how many problems there are. If there is a manageable amount of issues, they will be resolved, the number of pointers will be increased. If too many issues are present we should reassess.

We also need to make sure that multitouch testing doesn't bias toward always testing with multitouch. Some of the fuzzing needs to remain as single presses. We need to inspect the implementation of fuzzing to understand this.

jessegreenberg commented

We also need to make sure that multitouch testing doesn't bias toward always testing with multitouch. Some of the fuzzing needs to remain as single presses. We need to inspect the implementation of fuzzing to understand this.

Looking at InputFuzzer.fuzzEvents:

Every frame there is a chance that 0 - N actions will be made, where N default is currently 100. Actions are random in their selection, while making sure that the number of pointers down doesn't exceed ?fuzzPointers value and up events can only go down if there are active pointers.

I sampled 10,000 actions with ?fuzz&fuzzPointers=3 and the average number of pointers was 1.7. So most of the time there are multiple presses down. After fuzzing for about 30 seconds with ?fuzzPointers=2 the average number of pointers stabilized at 1.18. Still a slight bias to more than one, but better. Lets go with ?fuzzPointers=2 when we enable this.

jessegreenberg commented

When I run fuzzing with ?fuzzPointers=2, I only encounter these four errors:

balloons-and-static-electricity
Uncaught Error: Assertion failed: cannot set to grabbable if already set that way
Error: Assertion failed: cannot set to grabbable if already set that way
    at window.assertions.assertFunction (http://10.0.0.179:8080/assert/js/assert.js:22:13)
    at GrabDragInteraction.releaseDraggable (http://10.0.0.179:8080/scenery-phet/js/accessibility/GrabDragInteraction.js:443:15)
    at PressListener.release [as _releaseListener] (http://10.0.0.179:8080/scenery-phet/js/accessibility/GrabDragInteraction.js:393:16)
    at PressListener.onRelease (http://10.0.0.179:8080/scenery/js/listeners/PressListener.js:619:10)
    at Action.execute (http://10.0.0.179:8080/axon/js/Action.js:225:18)
    at PressListener.release (http://10.0.0.179:8080/scenery/js/listeners/PressListener.js:418:25)
    at PressListener.interrupt (http://10.0.0.179:8080/scenery/js/listeners/PressListener.js:475:12)
    at PressListener.pointerCancel (http://10.0.0.179:8080/scenery/js/listeners/PressListener.js:754:12)
    at Input.dispatchToListeners (http://10.0.0.179:8080/scenery/js/input/Input.js:1852:25)
    at Input.dispatchEvent (http://10.0.0.179:8080/scenery/js/input/Input.js:1806:10)
friction
Uncaught Error: Assertion failed: cannot set to grabbable if already set that way
Error: Assertion failed: cannot set to grabbable if already set that way
    at window.assertions.assertFunction (http://10.0.0.179:8080/assert/js/assert.js:22:13)
    at FrictionGrabDragInteraction.releaseDraggable (http://10.0.0.179:8080/scenery-phet/js/accessibility/GrabDragInteraction.js:443:15)
    at PressListener.release [as _releaseListener] (http://10.0.0.179:8080/scenery-phet/js/accessibility/GrabDragInteraction.js:393:16)
    at PressListener.onRelease (http://10.0.0.179:8080/scenery/js/listeners/PressListener.js:619:10)
    at Action.execute (http://10.0.0.179:8080/axon/js/Action.js:225:18)
    at PressListener.release (http://10.0.0.179:8080/scenery/js/listeners/PressListener.js:418:25)
    at PressListener.interrupt (http://10.0.0.179:8080/scenery/js/listeners/PressListener.js:475:12)
    at PressListener.pointerCancel (http://10.0.0.179:8080/scenery/js/listeners/PressListener.js:754:12)
    at Input.dispatchToListeners (http://10.0.0.179:8080/scenery/js/input/Input.js:1852:25)
    at Input.dispatchEvent (http://10.0.0.179:8080/scenery/js/input/Input.js:1806:10)
make-a-ten
Uncaught Error: Assertion failed: Did not find matching Node
Error: Assertion failed: Did not find matching Node
    at window.assertions.assertFunction (http://10.0.0.179:8080/assert/js/assert.js:22:13)
    at MakeATenGameScreenView.findPaperNumberNode (http://10.0.0.179:8080/make-a-ten/js/make-a-ten/common/view/MakeATenCommonView.js:147:15)
    at MakeATenGameScreenView.tryToCombineNumbers (http://10.0.0.179:8080/make-a-ten/js/make-a-ten/common/view/MakeATenCommonView.js:158:30)
    at DragListener.end [as _end] (http://10.0.0.179:8080/make-a-ten/js/make-a-ten/common/view/PaperNumberNode.js:89:7)
    at http://10.0.0.179:8080/scenery/js/listeners/DragListener.js:344:25
    at DragListener.onRelease (http://10.0.0.179:8080/scenery/js/listeners/PressListener.js:621:17)
    at Action.execute (http://10.0.0.179:8080/axon/js/Action.js:225:18)
    at DragListener.release (http://10.0.0.179:8080/scenery/js/listeners/PressListener.js:418:25)
    at DragListener.release (http://10.0.0.179:8080/scenery/js/listeners/DragListener.js:340:37)
    at DragListener.pointerUp (http://10.0.0.179:8080/scenery/js/listeners/PressListener.js:730:12)
masses-and-springs-basics
Uncaught Error: Assertion failed
Error: Assertion failed
    at window.assertions.assertFunction (http://10.0.0.179:8080/assert/js/assert.js:22:13)
    at Tandem.removeChild (http://10.0.0.179:8080/tandem/js/Tandem.js:292:15)
    at Tandem.dispose (http://10.0.0.179:8080/tandem/js/Tandem.js:300:23)
    at Tandem.removePhetioObject (http://10.0.0.179:8080/tandem/js/Tandem.js:211:25)
    at MutableOptionsNodeConstructor.dispose (http://10.0.0.179:8080/tandem/js/PhetioObject.js:556:19)
    at MutableOptionsNodeConstructor.dispose (http://10.0.0.179:8080/scenery/js/nodes/Node.js:5351:36)
    at MutableOptionsNodeConstructor.PhetioObject.dispose (http://10.0.0.179:8080/tandem/js/PhetioObject.js:199:20)
    at MutableOptionsNode.disposeCopy (http://10.0.0.179:8080/sun/js/MutableOptionsNode.js:106:26)
    at MutableOptionsNode.replaceCopy (http://10.0.0.179:8080/sun/js/MutableOptionsNode.js:95:12)
    at listener (http://10.0.0.179:8080/axon/js/Multilink.js:46:20)

The friction and BASE issues are likely the same problem, and I expect would likely show up in other sims with a11y.

EDIT: Another note, it looks like the masses-and-springs-basics issues is on CT with fuzzing only one pointer.

EDIT2: I ran the tests again, and saw same number of assertions thrown, but cannot set to grabbable if already set that way showed up in coulombs-law instead of friction.

jessegreenberg commented

phetsims/scenery-phet#622 resolves the GrabDragInteraction issues. the masses-and-springs-basics issue is handled in phetsims/masses-and-springs-basics#67. The only issue remaining is in make-a-ten.

jessegreenberg commented

A new one popped up

fractions-intro
Uncaught Error: Assertion failed
Error: Assertion failed
    at window.assertions.assertFunction (http://10.0.0.179:8080/assert/js/assert.js:22:13)
    at CircularSceneNode.getBucketPosition (http://10.0.0.179:8080/fractions-common/js/intro/view/ContainerSetScreenView.js:98:17)
    at http://10.0.0.179:8080/fractions-common/js/intro/view/CellSceneNode.js:222:54
    at DragListener.end [as _end] (http://10.0.0.179:8080/fractions-common/js/intro/view/PieceNode.js:81:18)
    at http://10.0.0.179:8080/scenery/js/listeners/DragListener.js:344:25
    at DragListener.onRelease (http://10.0.0.179:8080/scenery/js/listeners/PressListener.js:621:17)
    at Action.execute (http://10.0.0.179:8080/axon/js/Action.js:225:18)
    at DragListener.release (http://10.0.0.179:8080/scenery/js/listeners/PressListener.js:418:25)
    at DragListener.release (http://10.0.0.179:8080/scenery/js/listeners/DragListener.js:340:37)
    at DragListener.interrupt (http://10.0.0.179:8080/scenery/js/listeners/PressListener.js:475:12)
jessegreenberg commented

Running tests again locally with longer duration before committing to change because #106 (comment) did not appear first run through. Currently at 'B' sims with testDuration=60000 and now issues found.

jessegreenberg commented

Here is the new list after fuzzing for one minute per sim:

  • buoyancy: Uncaught Error: Assertion failed: Cubic start should be finite: Vector2(-Infinity, 0), see phetsims/buoyancy#7
  • buoyancy: Uncaught Error: Assertion failed: reentry detected, value=-0.14404020023346006, oldValue=-0.1244908943176274 (EDIT: reentry is probably a symptom of the previous error)
  • density: Uncaught Error: Assertion failed
  • fractions-intro: Uncaught Error: Assertion failed
  • fractions-mixed-numbers: Uncaught Error: Assertion failed
  • make-a-ten: Uncaught Error: Assertion failed: Did not find matching Node, see phetsims/make-a-ten#292
  • molecule-polarity: event.isFromPDOM is not a function
  • molecules-and-light: Assertion failed: matrix should be a finite Matrix3
  • projectile-motion: Uncaught Error: Assertion failed: timeToGround: NaN, previousPoint.velocity: Vector2(0, 0), previousPoint.acceleration: Vector2(0, 0), fromIf: true (pre-existing, says @zepumph)
  • vector-addition: Uncaught Error: Assertion failed: Cannot drag tip when not on graph, (see phetsims/vector-addition#271)

EDIT: Converting to checkboxes.

jessegreenberg commented

Looks like the remaining issues are specific to each sim. Putting on developer meeting to review. In my opinion the remaining list is pretty short so it seems worth working on so we can fuzz with multiple pointers, and also test zoom feature with fuzzing.

  • 👍1
zepumph commented

@jessegreenberg will make issues in the above repos, and then we will turn this on after those are sorted out.

pixelzoom commented

There has been no movement on this issue, and sim-specific issues have not been created. Assigning to @ariel-phet and labeling for developer meeting to decide how to proceed. My recommendation is to enabled multi-touch fuzz testing, and have responsible devs fix their sims.

ariel-phet commented

@jessegreenberg will be moving this issue forward @pixelzoom

jessegreenberg commented

Issues have been made for the list in #106 (comment). I am running aqua again locally with ?fuzz&fuzzPointers=2&testDuration=30000, in case anything new has been introduced since last checked. At 'f' sims so far and assertions are the same.

jessegreenberg commented

OK, fuzz testing finished and the list of issues is the same. Issues have been created for all. 5 sims in total have issues. Would devs like this to be enabled now, or would you prefer to wait until these are resolved before changing CT?

pixelzoom commented

@jessegreenberg can you please post the query parameters used for the failures that you noted in #106 (comment)?

jessegreenberg commented

The query parameters used were the ones used by aqua:
ea&audioVolume=0&sound=disabled&testDuration=30000&testConcurrentBuilds=4&brand=phet&fuzz&fuzzPointers=2

Though I think the important ones are
ea&fuzz&fuzzPointers=2

pixelzoom commented

Would devs like this to be enabled now, or would you prefer to wait until these are resolved before changing CT?

My preference is to enable this now, and address the problems now. Otherwise we'll have to continue to iterate on this again, possibly with new problems on each iteration.

  • 👍2
zepumph commented

At dev meeting today, we decided to just add it, and handle the errors as they appear on CT.

pixelzoom commented

2/4/21 dev meeting:

We noted that there are some new CT issues related to this, and decided to create sim-specific issues to address them.

@jessegreenberg will add an option to phetmarks for this feature, then can close this.

jessegreenberg commented

I added a checkbox for fuzzPointers=2 to phetmarks. This can be closed.