search

phetsims / sun

User-interface components for PhET simulations, built on top of Scenery.
MIT License
4 stars 12 forks source link

CT: Designed API changes detected #744

Closed pixelzoom closed 2 years ago

pixelzoom commented 2 years ago

CT is complaining about iO API change. Regression? Intentional?

sun : phet-io-api-compatibility : unbuilt
https://bayes.colorado.edu/continuous-testing/ct-snapshots/1646620780268/sun/sun_en.html?continuousTest=%7B%22test%22%3A%5B%22sun%22%2C%22phet-io-api-compatibility%22%2C%22unbuilt%22%5D%2C%22snapshotName%22%3A%22snapshot-1646620780268%22%2C%22timestamp%22%3A1646624557006%7D&ea&brand=phet-io&phetioStandalone&phetioCompareAPI&randomSeed=332211
Query: ea&brand=phet-io&phetioStandalone&phetioCompareAPI&randomSeed=332211
Uncaught Error: Assertion failed: Designed API changes detected, please roll them back or revise the reference API:

PhET-iO Element missing: sun.componentsScreen.view.demoHSlider.enabledRangeProperty

PhET-iO Element missing: sun.componentsScreen.view.demoVSlider.enabledRangeProperty
Error: Assertion failed: Designed API changes detected, please roll them back or revise the reference API:

PhET-iO Element missing: sun.componentsScreen.view.demoHSlider.enabledRangeProperty

PhET-iO Element missing: sun.componentsScreen.view.demoVSlider.enabledRangeProperty
at window.assertions.assertFunction (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1646620780268/assert/js/assert.js:25:13)
at XMLHttpRequest.<anonymous> (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1646620780268/chipper/dist/js/phet-io/js/phetioEngine.js:319:23)
id: Bayes Chrome
Snapshot from 3/6/2022, 7:39:40 PM
pixelzoom commented 2 years ago

In Slack, @zepumph said:

Likely related to @jonathanolson's typescript refactors over the weekend. We met on Friday and discussed some linked element stuff, but I'm pretty sure we don't expect any changes to the API from this.

zepumph commented 2 years ago

Hmm, I wonder if this is actually a bug fixed by converting to typescript. Initial investigation shows that a linked element for enabledRangeProperty was removed from the API, but it was tied to an uninstrumented Slider, so maybe this is correct? I'm not really sure, but thought I'd state the initial investigation here.

https://github.com/phetsims/sun/blob/073826d96aee4193d78c10174fa645c66b9fbe9d/js/demo/ComponentsScreenView.js#L285

zepumph commented 2 years ago

@jonathanolson and I looked into this, we can't figure out why or how an uninstrumented enabledRangeProperty was added to the API. After the refactor we see that it is correctly removed. Closing