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.

Sign up for GitHub

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

Jump to bottom

Enable multitouch testing on CT #106

Closed
jessegreenberg opened this issue Aug 27, 2020 · 18 comments
Closed

Enable multitouch testing on CT #106

jessegreenberg opened this issue Aug 27, 2020 · 18 comments
Assignees
@jessegreenberg
Labels
dev:enhancement

Comments

@jessegreenberg
Copy link
Contributor

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 jessegreenberg self-assigned this Aug 27, 2020
@jessegreenberg
Copy link
Contributor Author

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

jessegreenberg commented Aug 31, 2020 •
edited
Loading

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 jessegreenberg mentioned this issue Aug 31, 2020
Closed
@jessegreenberg
Copy link
Contributor Author

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 jessegreenberg mentioned this issue Aug 31, 2020
Closed
@jessegreenberg
Copy link
Contributor Author

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 jessegreenberg mentioned this issue Aug 31, 2020
Open
@jessegreenberg
Copy link
Contributor Author

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

jessegreenberg commented Sep 1, 2020 •
edited by pixelzoom
Loading

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 ?fuzzPointers=2 error: "Cubic start should be finite: Vector2(-Infinity, 0)" 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 CT: Did not find matching Node 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 ?fuzzPointers=2 error: "Cannot drag tip when not on graph" vector-addition#271)

EDIT: Converting to checkboxes.

This was referenced Sep 1, 2020
Closed
Closed
@jessegreenberg
Copy link
Contributor Author

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.

@jessegreenberg jessegreenberg removed their assignment Sep 1, 2020
@jessegreenberg jessegreenberg mentioned this issue Sep 3, 2020
Closed
@zepumph
Copy link
Member

zepumph commented Sep 17, 2020

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

@zepumph zepumph mentioned this issue Oct 21, 2020
Closed
17 tasks
@pixelzoom
Copy link
Contributor

pixelzoom commented Nov 11, 2020 •
edited
Loading

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.

@jessegreenberg jessegreenberg mentioned this issue Nov 19, 2020
Closed
@ariel-phet ariel-phet removed their assignment Nov 19, 2020
@ariel-phet
Copy link

@jessegreenberg will be moving this issue forward @pixelzoom

@jessegreenberg jessegreenberg mentioned this issue Nov 19, 2020
Closed
@jessegreenberg
Copy link
Contributor Author

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

jessegreenberg commented Nov 19, 2020 •
edited
Loading

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

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

@jessegreenberg
Copy link
Contributor Author

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

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.

@zepumph
Copy link
Member

zepumph commented Dec 3, 2020

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

jessegreenberg added a commit to phetsims/perennial that referenced this issue Feb 3, 2021
@jessegreenberg
add multitouch-fuzz tests with 2 pointers to CT, see phetsims/aqua#106
e10047f
@pixelzoom
Copy link
Contributor

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 added a commit to phetsims/phetmarks that referenced this issue Feb 5, 2021
@jessegreenberg
Add a fuzzPointers=2 checkbox to phetmarks, only usable when fuzz is …
4f2b24c 
…enabled - see phetsims/aqua#106
@jessegreenberg
Copy link
Contributor Author

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

This was referenced Feb 9, 2021
Closed
Closed
Open
Open
Closed
Closed
Closed
Open
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jessegreenberg jessegreenberg

Labels
dev:enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@pixelzoom @ariel-phet @jessegreenberg @zepumph