phetsims / equality-explorer

"Equality Explorer" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
2 stars 3 forks source link

CT: value failed isValidValue: -70 #157

Closed pixelzoom closed 4 years ago

pixelzoom commented 4 years ago

CT is failing consistently with errors like the one below. This is likely related to NumberProperty range changes in https://github.com/phetsims/axon/issues/274.

equality-explorer : xss-fuzz : run
Query: brand=phet&ea&fuzz&stringTest=xss&memoryLimit=1000
Uncaught Error: Assertion failed: value failed isValidValue: -70
Error: Assertion failed: value failed isValidValue: -70
    at window.assertions.assertFunction (https://bayes.colorado.edu/continuous-testing/snapshot-1579884041681/assert/js/assert.js?bust=1579888180635:22:13)
    at Object.isValueValid (https://bayes.colorado.edu/continuous-testing/snapshot-1579884041681/axon/js/ValidatorDef.js?bust=1579888180718:301:41)
    at validate (https://bayes.colorado.edu/continuous-testing/snapshot-1579884041681/axon/js/validate.js?bust=1579888180718:30:20)
    at NumberProperty.assertNumberPropertyValidateValue (https://bayes.colorado.edu/continuous-testing/snapshot-1579884041681/axon/js/NumberProperty.js?bust=1579888180718:94:11)
    at https://bayes.colorado.edu/continuous-testing/snapshot-1579884041681/axon/js/NumberProperty.js?bust=1579888180718:126:14
    at TinyEmitter.emit (https://bayes.colorado.edu/continuous-testing/snapshot-1579884041681/axon/js/TinyEmitter.js?bust=1579888180718:68:55)
    at NumberProperty._notifyListeners (https://bayes.colorado.edu/continuous-testing/snapshot-1579884041681/axon/js/Property.js?bust=1579888180718:279:27)
    at NumberProperty.set (https://bayes.colorado.edu/continuous-testing/snapshot-1579884041681/axon/js/Property.js?bust=1579888180718:178:16)
    at NumberProperty.set value [as value] (https://bayes.colorado.edu/continuous-testing/snapshot-1579884041681/axon/js/Property.js?bust=1579888180718:349:34)
    at SolveItScene.nextChallenge (https://bayes.colorado.edu/continuous-testing/snapshot-1579884041681/equality-explorer/js/solveit/model/SolveItScene.js?bust=1579888180718:149:42)
id: Bayes Chrome
Approximately 1/24/2020, 9:40:41 AM
pixelzoom commented 4 years ago

Reproduced manually with ?ea&screens=5&fuzz&log. It doesn't happen immediately, so let it run awhile.

The problem is that xVariable (x) for SolveItScene has range [-40,40]. But "level 1, type 3" challenges (for example) derive the value of xVariable. Based on the range of coefficients a, b, c, d, x value derived by the challenge generator can be outside that range. Having a range for x in challenges is generally inappropriate, because x is not controlled by the user, and the range of x depends on the form of the equation and the range of its coefficients.

pixelzoom commented 4 years ago

Now that NumberProperty is more strict about its range, this issue and #156 popped up.

There were 3 problems here: (1) all "variables" (Variable.js) had a default range, (2) the default was not generally appropriate, and (3) ranges were set for variables that shouldn't have a range, for example the derived value of 'x' in certain game challenges.

The only variables that require a range are those that are user-controlled (via pickers). So the gist of the above commits is that Variable.js defaults to range: null, and those scenes that control variable values via pickers then set an appropriate range.

This affects all 3 sims in the equality-explorer family. I tested them all manually and with ?ea&fuzz.

I'll leave this open until CT verifies.

pixelzoom commented 4 years ago

CT is happy for 3 cycles, closing.

zepumph commented 4 years ago

Thanks for taking care of this!