phetsims / keplers-laws

"Kepler's Laws" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
1 stars 1 forks source link

CT: Property value not valid: Failed validation for validators[0]: Should not be NaN #171

Closed pixelzoom closed 1 year ago

pixelzoom commented 1 year ago
keplers-laws : fuzz : unbuilt
http://128.138.93.172/continuous-testing/ct-snapshots/1695137275091/keplers-laws/keplers-laws_en.html?continuousTest=%7B%22test%22%3A%5B%22keplers-laws%22%2C%22fuzz%22%2C%22unbuilt%22%5D%2C%22snapshotName%22%3A%22snapshot-1695137275091%22%2C%22timestamp%22%3A1695143068149%7D&brand=phet&ea&fuzz
Query: brand=phet&ea&fuzz
Uncaught Error: Assertion failed: validation failed:
Property value not valid: Failed validation for validators[0]: Should not be NaN: value failed isValidValue: NaN
prunedValidator:
{
"valueType": "number",
"phetioType": {
"typeName": "NumberIO",
"supertype": {
"typeName": "ObjectIO",
"documentation": "The root of the IO Type hierarchy",
"methods": {},
"events": [],
"metadataDefaults": {
"phetioTypeName": "ObjectIO",
"phetioDocumentation": "",
"phetioState": true,
"phetioReadOnly": false,
"phetioEventType": "MODEL",
"phetioHighFrequency": false,
"phetioPlayback": false,
"phetioDynamicElement": false,
"phetioIsArchetype": false,
"phetioFeatured": false,
"phetioDesigned": false,
"phetioArchetypePhetioID": null
},
"dataDefaults": {
"initialState": null
},
"methodOrder": [],
"parameterTypes": [],
"validator": {
"validationMessage": "Validation failed IOType Validator: ObjectIO"
},
"defaultDeserializationMethod": "fromStateObject",
"stateSchema": null,
"isFunctionType": false
},
"documentation": "IO Type for Javascript's number primitive type",
"methods": {},
"events": [],
"metadataDefaults": {},
"dataDefaults": {},
"methodOrder": [],
"parameterTypes": [],
"validator": {
"valueType": "number",
"validationMessage": "Validation failed IOType Validator: NumberIO"
},
"defaultDeserializationMethod": "fromStateObject",
"stateSchema": {
"displayString": "number",
"validator": {},
"compositeSchema": null
},
"isFunctionType": false
},
"validators": [
{
"validationMessage": "Should not be NaN"
},
{
"validationMessage": "Number must be within rangeProperty value."
}
]
}
Error: Assertion failed: validation failed:
at window.assertions.assertFunction (http://128.138.93.172/continuous-testing/ct-snapshots/1695137275091/assert/js/assert.js:28:13)
at assert (validate.ts:26:16)
at validate (ReadOnlyProperty.ts:296:14)
at _notifyListeners (ReadOnlyProperty.ts:262:13)
at unguardedSet (ReadOnlyProperty.ts:246:11)
at set (Property.ts:54:10)
at (EllipticalOrbitEngine.ts:247:32)
at updateBodyDistances (EllipticalOrbitEngine.ts:306:9)
at update (EllipticalOrbitEngine.ts:188:13)
at callback (Multilink.ts:111:10)
[URL] http://128.138.93.172/continuous-testing/aqua/html/sim-test.html?url=..%2F..%2Fct-
...

id: "Sparky Node Puppeteer"
Snapshot from 9/19/2023, 9:27:55 AM
pixelzoom commented 1 year ago

In Slack#DM, @AgustinVallejo asked me:

I'm not sure at all how https://github.com/phetsims/keplers-laws/issues/171 would be happening. After examining the call stack, it seems like for that reported value to be a NaN, things should've broken way before reaching that point. Not sure what you'd recommend for that case? I'm also not sure how I would replicate it

I'm not sure how to proceed here. I don't know why the error message includes JSON that looks like it's from NumberIO. I recommend that you consult with @zepumph, since he's more familiar with validation and its PhET-iO aspects.

zepumph commented 1 year ago

This is just validation code working as desired. If you look at the call stack, you set a property that says "no NaN values please" to NaN. One of these calls:

https://github.com/phetsims/keplers-laws/blob/1034f4edcb6e37eb86ca1394fc480115bb6627fc/js/common/model/EllipticalOrbitEngine.ts#L247-L248

I wasn't able to reproduce in 3 minutes of fuzzing. @AgustinVallejo let me know if you want to discuss more.

AgustinVallejo commented 1 year ago

@zepumph that's what I figured, but to me it's weird that it happened in line 247 since for any of these values to become NaN, assertions should've triggered way before. And I haven't been able to reproduce either.

KatieWoe commented 1 year ago

I didn't see any problems in the spot check when fuzzing on the debug version. If there is another way to test this, let me know.

AgustinVallejo commented 1 year ago

Nope, not that I know of. Should I close then?

KatieWoe commented 1 year ago

If there's nothing else for us to look out for, sure.