search

phetsims / aqua

Automatic QUality Assurance
MIT License
2 stars 4 forks source link

Focus and blur events don't fire while fuzzing if the document does not have focus #134

Open jessegreenberg opened 2 years ago

jessegreenberg commented 2 years ago

If the document does not have focus, focus and blur events don't fire. Code dependent on these listeners will never fire while the sim is in the background. For example, that means that FocusManager.pdomFocusProperty will always be null because it is set in response to focus and blur events.

This is causing https://github.com/phetsims/geometric-optics/issues/384. I also ran into it this morning while working on KeyboardDragListener, seeing that none of interrupt from blur was never called while fuzzing which is a problem.

jessegreenberg commented 2 years ago

@zepumph and I met to discuss. We tried to get a headless Chrome up and running to verify that focus and blur events do not fire on that platform and to find a workaround but we weren't able to get it running successfully. But we did find https://github.com/puppeteer/puppeteer/issues/1462 which confirms that focus and blur won't fire in headless chrome/puppeteer.

We couldn't think of fix yet so we are going to add a check to only fuzz keyboard events if document.hasFocus() is true. That means that ?keyboardFuzz will only fuzz events if the sim is in the foreground and will do no fuzzing on CT.

We want to try again to get focus and blur events in headless chrome somehow so we can fuzz on CT again. If that is not possible we will need to hit the drawing board and maybe think of a scenery level abstraction to recreate focus and blur in this environment.

jessegreenberg commented 2 years ago

I am seeing errors from fuzzBoard on CT still, I am curious how that is possible after the above commit. Perhaps my understanding of this issue is not correct.

Query: brand=phet&ea&fuzzBoard&voicingInitiallyEnabled&memoryLimit=1000
Uncaught Error: Assertion failed: there should be two pairs of equal opposite angles for a parallelogram
Error: Assertion failed: there should be two pairs of equal opposite angles for a parallelogram
at window.assertions.assertFunction (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1648721107869/assert/js/assert.js:25:13)
at QuadrilateralDescriber.getSecondDetailsStatement (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1648721107869/chipper/dist/js/quadrilateral/js/quadrilateral/view/QuadrilateralDescriber.js:435:19)
at QuadrilateralScreenView.getVoicingDetailsContent (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1648721107869/chipper/dist/js/quadrilateral/js/quadrilateral/view/QuadrilateralScreenView.js:215:57)
at VoicingToolbarAlertManager.createDetailsContent (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1648721107869/chipper/dist/js/joist/js/toolbar/VoicingToolbarAlertManager.js:42:23)
at LabelButtonRow.playContent (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1648721107869/chipper/dist/js/joist/js/toolbar/VoicingToolbarItem.js:194:35)
at https://bayes.colorado.edu/continuous-testing/ct-snapshots/1648721107869/chipper/dist/js/joist/js/toolbar/VoicingToolbarItem.js:88:13
at TinyProperty.emit (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1648721107869/chipper/dist/js/axon/js/TinyEmitter.js:68:9)
at BooleanProperty._notifyListeners (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1648721107869/chipper/dist/js/axon/js/Property.js:227:23)
at BooleanProperty.set (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1648721107869/chipper/dist/js/axon/js/Property.js:156:14)
at BooleanProperty.set value [as value] (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1648721107869/chipper/dist/js/axon/js/Property.js:297:10)
id: Bayes Chrome
Snapshot from 3/31/2022, 6:05:07 AM
jessegreenberg commented 2 years ago

Perhaps we can just explicitly put focus on the frame where the sim is being fuzzed to ensure that it will produce focus and blur events. It seems like it is sometimes doing that but not always. Like https://github.com/phetsims/phet-io-wrappers/blob/17e332da4b14b43f1a54f7f00e87ffa278e05ec1/input-record-and-playback/inputRecordAndPlayback.js#L229

jessegreenberg commented 1 year ago

This came up again in https://github.com/phetsims/joist/issues/897. Since puppeteer doesn't support focus and blur events, unit tests for focus state were failing. Wrapping the tests in a document.hasFocus() check lets us skip the tests on puppeteer but it would be better to find a way to keep running these tests automated tests. IF that is impossible maybe a way to generalize the hasFocus check.

jessegreenberg commented 1 year ago

This is happening again on CT for a new set of focus tests:

scenery : top-level-unit-tests : unbuilt?ea&brand=phet-io
http://127.0.0.1/continuous-testing/ct-snapshots/1685007883249/scenery/scenery-tests.html?ea&brand=phet-io
393 out of 399 tests passed. 6 failed.
ParallelDOMTests: replaceChild failed:
f has focus before being replaced
at Object.<anonymous> (http://127.0.0.1/continuous-testing/ct-snapshots/1685007883249/chipper/dist/js/scenery/js/accessibility/pdom/ParallelDOMTests.js:1143:10)

ParallelDOMTests: replaceChild failed:
testNode has focus after replacing focused node f
at Object.<anonymous> (http://127.0.0.1/continuous-testing/ct-snapshots/1685007883249/chipper/dist/js/scenery/js/accessibility/pdom/ParallelDOMTests.js:1150:10)

ParallelDOMTests: replaceChild failed:
browser is focusing testNode
at Object.<anonymous> (http://127.0.0.1/continuous-testing/ct-snapshots/1685007883249/chipper/dist/js/scenery/js/accessibility/pdom/ParallelDOMTests.js:1151:10)

ParallelDOMTests: replaceChild failed:
d has focus before being replaced
at Object.<anonymous> (http://127.0.0.1/continuous-testing/ct-snapshots/1685007883249/chipper/dist/js/scenery/js/accessibility/pdom/ParallelDOMTests.js:1166:10)

ParallelDOMTests: swapVisibility failed:
c should now have focus after swapVisibility
at Object.<anonymous> (http://127.0.0.1/continuous-testing/ct-snapshots/1685007883249/chipper/dist/js/scenery/js/accessibility/pdom/ParallelDOMTests.js:1442:10)

ParallelDOMTests: swapVisibility failed:
c should now have focus after swapVisibility
at Object.<anonymous> (http://127.0.0.1/continuous-testing/ct-snapshots/1685007883249/chipper/dist/js/scenery/js/accessibility/pdom/ParallelDOMTests.js:1453:10)

ParallelDOMTests: focusable option failed:
focusable div should be focused after calling focus()
at Object.<anonymous> (http://127.0.0.1/continuous-testing/ct-snapshots/1685007883249/chipper/dist/js/scenery/js/accessibility/pdom/ParallelDOMTests.js:1546:10)

ParallelDOMTests: move to front/move to back failed:
b should have focus after a moved to front
at Object.<anonymous> (http://127.0.0.1/continuous-testing/ct-snapshots/1685007883249/chipper/dist/js/scenery/js/accessibility/pdom/ParallelDOMTests.js:1856:10)
...

I will opt out of these for now when the document is not active.

jessegreenberg commented 1 year ago

More tests have been failing. I added the opt out to many more tests. I looked for a more elegant way to opt out in a beforeEach or something but did not find better support from QUnit.

Now when I run scenery unit tests while the browser is in the background everything is passing. If this is ever resolved we should remove these opt-outs.