CT: tried to removeListener on something that wasn't a listener

[pixelzoom]

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.

gene-expression-essentials : multitouch-fuzz : unbuilt
https://bayes.colorado.edu/continuous-testing/ct-snapshots/1612841514933/gene-expression-essentials/gene-expression-essentials_en.html?continuousTest=%7B%22test%22%3A%5B%22gene-expression-essentials%22%2C%22multitouch-fuzz%22%2C%22unbuilt%22%5D%2C%22snapshotName%22%3A%22snapshot-1612841514933%22%2C%22timestamp%22%3A1612844859673%7D&brand=phet&ea&fuzz&fuzzPointers=2&memoryLimit=1000
Query: brand=phet&ea&fuzz&fuzzPointers=2&memoryLimit=1000
Uncaught Error: Assertion failed: tried to removeListener on something that wasn't a listener
Error: Assertion failed: tried to removeListener on something that wasn't a listener
at window.assertions.assertFunction (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1612841514933/assert/js/assert.js:25:13)
at TinyEmitter.removeListener (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1612841514933/axon/js/TinyEmitter.js:120:7)
at BooleanProperty.unlink (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1612841514933/axon/js/Property.js:420:25)
at TranscriptionFactorCreatorNode.reset (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1612841514933/gene-expression-essentials/js/manual-gene-expression/view/BiomoleculeCreatorNode.js:102:47)
at BiomoleculeToolboxNode.reset (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1612841514933/gene-expression-essentials/js/manual-gene-expression/view/BiomoleculeToolboxNode.js:201:44)
at https://bayes.colorado.edu/continuous-testing/ct-snapshots/1612841514933/gene-expression-essentials/js/manual-gene-expression/view/ManualGeneExpressionScreenView.js:263:34
at Array.forEach (<anonymous>)
at listener (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1612841514933/gene-expression-essentials/js/manual-gene-expression/view/ManualGeneExpressionScreenView.js:262:36)
at ResetAllButton.options.listener (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1612841514933/scenery-phet/js/buttons/ResetAllButton.js:59:7)
at TinyEmitter.emit (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1612841514933/axon/js/TinyEmitter.js:71:18)
id: Bayes Chrome
Snapshot from 2/8/2021, 8:31:54 PM

[jbphet]

Something is a bit weird here. This problem is no longer showing up in CT, as can be seen in the screenshot below. It also doesn't occur when I test with the multi-touch query parameters on my local machine. However, after some investigation, I'm able to duplicate the problem manually on the master version. Here is the sequence:

• On a touch-based device, load the sim and go to the first screen
• With a finger, drag the transcription factor out from the biomolecule tool box at the upper left
• While continuing to hold onto the transcription factor, press the reset button

I'll fix this, but this situation makes me suspect that the multi-touch testing isn't working correctly, and I'll need to follow up on that.

Here's a screenshot to show that the sim with the errant code still in place is passing the multi-touch automated testing:

[jbphet]

I updated to the most recent drag handling and forwarding approaches, and can no longer reproduce the problem manually. Also, I didn't see any issues after two minutes each of fuzz and multi-touch-fuzz testing. I'll leave this open to verify that CT is clear, and I'll also mark for dev meeting to discuss whether something is wrong with multi-touch fuzz testing, see above.

[jbphet]

CT was clear over the weekend.

[pixelzoom]

I think you still have a potential problem here. You’re calling userControlledProperty.unlink in 2 places in BiomoleculeCreatorNode. So it’s possible that userControlledPropertyObserver may not be a listener when reset is called -- or at best, it's unclear if userControlledPropertyObserver is guaranteed to be an observer when reset is called. To be safe, in reset I would check userControlledProperty.hasListener before doing the unlink.

[jessegreenberg]

I think the reason CT looks green is just because it usually isnt running much fuzzing at the moment, can see here that multitouch fuzzing especially is not running:

Maybe the priorities of certain tests can be adjusted to make sure that important fuzzing is always run before moving on to next set of SHAs.

[jbphet]

Above, @pixelzoom said:

I think you still have a potential problem here. You’re calling userControlledProperty.unlink in 2 places in BiomoleculeCreatorNode...

I didn't want to add the suggested hasListener test since the two code paths that do the unlink are intended to be mutually exclusive. I stated this in a Slack discussion, and @pixelzoom recommended adding an assert to make sure that this assumption is always true, so that's what I've done.

[mattpen]

Dev Meeting 3/13: @jonathanolson is looking into the issues with multitouch fuzz testing on CT.

[jbphet]

CT has continued to be clear during this time, and we've discussed the issue with CT in dev meeting, so I'm going to close.