phetsims / arithmetic

"Arithmetic" is an educational simulation in HTML5, by PhET Interactive Simulations.
http://phet.colorado.edu/en/simulation/arithmetic
GNU General Public License v3.0
5 stars 5 forks source link

Automated testing error #157

Closed jonathanolson closed 7 years ago

jonathanolson commented 8 years ago
TypeError: Cannot read property '0' of undefined
    at Array. (http://localhost:8080/arithmetic/js/divide/view/DivideScreenTableNode.js?bust=1449609646198:40:73)
    at Property.inherit._notifyObservers (http://localhost:8080/axon/js/Property.js?bust=1449609646198:130:29)
    at Property.inherit._setAndNotifyObservers (http://localhost:8080/axon/js/Property.js?bust=1449609646198:116:14)
    at Property.inherit.set (http://localhost:8080/axon/js/Property.js?bust=1449609646198:84:16)
    at DivideModel.Object.defineProperty.set [as state] (http://localhost:8080/axon/js/PropertySet.js?bust=1449609646198:141:43)
    at DivideModel.inherit.restoreGameEnvironment (http://localhost:8080/arithmetic/js/common/model/ArithmeticModel.js?bust=1449609646198:288:18)
    at DivideModel.inherit.setLevel (http://localhost:8080/arithmetic/js/common/model/ArithmeticModel.js?bust=1449609646198:222:14)
    at ArithmeticView.LevelSelectionNode.centerX (http://localhost:8080/arithmetic/js/common/view/ArithmeticView.js?bust=1449609646198:48:33)
    at LevelSelectionButton.buttonWidth (http://localhost:8080/arithmetic/js/common/view/LevelSelectionNode.js?bust=1449609646198:94:11)
    at http://localhost:8080/sun/js/buttons/PushButtonModel.js?bust=1449609646198:130:9
jonathanolson commented 8 years ago

Doesn't seem to always reproduce.

jbphet commented 8 years ago

Good catch Fuzz Test (and, by extension, @jonathanolson)! This issue was caused when a large value was entered for the multiplicand on one of the Divide levels, then the back button is pressed, then the user returns to the same level. While it is probably unlikely that real users would do this, it did identify a problem that the multiplicand should be reset in this situation. Code to do so has been added, and the issue appears to be fixed. Closing.

phet-steele commented 7 years ago

@jbphet this error has occurred again during 11/15 automated testing. I could not necessarily repro the problem by hand using the above procedure.

TypeError: Cannot read property '0' of undefined
    at Array. (http://phettest.colorado.edu/arithmetic/js/divide/view/DivideScreenTableNode.js:41:73)
    at Emitter.emit2 (http://phettest.colorado.edu/axon/js/Emitter.js:139:49)
    at Property._notifyObservers (http://phettest.colorado.edu/axon/js/Property.js:170:29)
    at Property._setAndNotifyObservers (http://phettest.colorado.edu/axon/js/Property.js:158:14)
    at Property.set (http://phettest.colorado.edu/axon/js/Property.js:126:16)
    at DivideModel.set [as state] (http://phettest.colorado.edu/axon/js/PropertySet.js:149:43)
    at DivideModel.retryProblem (http://phettest.colorado.edu/arithmetic/js/common/model/ArithmeticModel.js:156:18)
    at Array. (http://phettest.colorado.edu/arithmetic/js/common/view/WorkspaceNode.js:185:17)
    at Emitter.emit2 (http://phettest.colorado.edu/axon/js/Emitter.js:139:49)
    at Property._notifyObservers (http://phettest.colorado.edu/axon/js/Property.js:170:29)
    at Property._setAndNotifyObservers (http://phettest.colorado.edu/axon/js/Property.js:158:14)
    at Property.set (http://phettest.colorado.edu/axon/js/Property.js:126:16)
    at DivideModel.set [as input] (http://phettest.colorado.edu/axon/js/PropertySet.js:149:43)
jonathanolson commented 7 years ago

Also occurred, once in require.js, once in built mode.

jbphet commented 7 years ago

The problem was due to a case in which the sim was trying to retry a problem, and the user had entered a high value for the multiplicand, and when the code tried to restore the AWAITING_USER_INPUT state, it tried to highlight the cell associated with that multiplicand, and it would index outside of the array that represents the multiplication table.

I found a way to reproduce it manually:

  1. Go to the Divide screen
  2. Play until there is a problem where the multiplicand (the leftmost term in the equation) needs to be filled in
  3. Enter a large 3-digit value, e.g. 921
  4. Press the "check" button (you'll get feedback indicating that the answer is incorrect)
  5. Press the backspace key once
  6. Press the "Check" button again

The code was deciding which row or column cell to highlight based upon which one had a value. This is flawed, since there are clearly cases where out-of-bounds values can come up (although such cases are rare). I've changed it to use the model value of activeInput to decide which cell to highlight.

jbphet commented 7 years ago

I fuzz tested on just the "Divide" screen for about 10 minutes, then fuzz tested the entire sim for about 15 minutes, and did not encounter any problems. I think we're good here. Closing.

samreid commented 7 years ago

I added another fix in 727526753ed84f4dd82d7ea2a05a92a62b21b75d, @jbphet can you please verify?

jbphet commented 7 years ago

Yes, that was needed. Thanks. Closing.