Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CT Designed API changes detected, please roll them back or revise the reference API: #161

Closed
KatieWoe opened this issue Apr 12, 2023 · 8 comments

Comments

@KatieWoe
Copy link

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

density.mysteryScreen.model.scale._data.initialState differs.
Expected:
{"canMove":false,"canRotate":false,"force":{"x":0,"y":0},"massShape":"BLOCK","matrix":{"entries":[1,0,0,0,1,0,-0.7410253438317398,0.03,1],"type":"TRANSLATION_2D"},"originalMatrix":{"entries":[1,0,0,0,1,0,-0.7410253438317398,0.03,1],"type":"TRANSLATION_2D"},"position":{"x":-0.7410253524780274,"y":0.030000001192092896},"stepMatrix":{"entries":[1,0,0,0,1,0,0,0,1],"type":"IDENTITY"},"tag":"NONE","velocity":{"x":0,"y":0}}
actual:
{"matrix":{"entries":[1,0,0,0,1,0,-0.74102534383174,0.03,1],"type":"TRANSLATION_2D"},"stepMatrix":{"entries":[1,0,0,0,1,0,0,0,1],"type":"IDENTITY"},"originalMatrix":{"entries":[1,0,0,0,1,0,-0.74102534383174,0.03,1],"type":"TRANSLATION_2D"},"canRotate":false,"canMove":false,"tag":"NONE","massShape":"BLOCK","position":{"x":-0.7410253524780274,"y":0.030000001192092896},"velocity":{"x":0,"y":0},"force":{"x":0,"y":0}}

Error: Assertion failed: Designed API changes detected, please roll them back or revise the reference API:
at window.assertions.assertFunction (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1681254360470/assert/js/assert.js:28:13)
at assert (phetioEngine.ts:365:22)
id: Bayes Puppeteer
Snapshot from 4/11/2023, 5:06:00 PM

----------------------------------

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

density.mysteryScreen.model.scale._data.initialState differs.
Expected:
{"canMove":false,"canRotate":false,"force":{"x":0,"y":0},"massShape":"BLOCK","matrix":{"entries":[1,0,0,0,1,0,-0.7410253438317398,0.03,1],"type":"TRANSLATION_2D"},"originalMatrix":{"entries":[1,0,0,0,1,0,-0.7410253438317398,0.03,1],"type":"TRANSLATION_2D"},"position":{"x":-0.7410253524780274,"y":0.030000001192092896},"stepMatrix":{"entries":[1,0,0,0,1,0,0,0,1],"type":"IDENTITY"},"tag":"NONE","velocity":{"x":0,"y":0}}
actual:
{"matrix":{"entries":[1,0,0,0,1,0,-0.74102534383174,0.03,1],"type":"TRANSLATION_2D"},"stepMatrix":{"entries":[1,0,0,0,1,0,0,0,1],"type":"IDENTITY"},"originalMatrix":{"entries":[1,0,0,0,1,0,-0.74102534383174,0.03,1],"type":"TRANSLATION_2D"},"canRotate":false,"canMove":false,"tag":"NONE","massShape":"BLOCK","position":{"x":-0.7410253524780274,"y":0.030000001192092896},"velocity":{"x":0,"y":0},"force":{"x":0,"y":0}}

Error: Assertion failed: Designed API changes detected, please roll them back or revise the reference API:
at window.assertions.assertFunction (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1681254360470/assert/js/assert.js:28:13)
at assert (phetioEngine.ts:365:22)
id: Bayes Puppeteer
Snapshot from 4/11/2023, 5:06:00 PM

----------------------------------

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

density.mysteryScreen.model.scale._data.initialState differs.
Expected:
{"canMove":false,"canRotate":false,"force":{"x":0,"y":0},"massShape":"BLOCK","matrix":{"entries":[1,0,0,0,1,0,-0.7410253438317398,0.03,1],"type":"TRANSLATION_2D"},"originalMatrix":{"entries":[1,0,0,0,1,0,-0.7410253438317398,0.03,1],"type":"TRANSLATION_2D"},"position":{"x":-0.7410253524780274,"y":0.030000001192092896},"stepMatrix":{"entries":[1,0,0,0,1,0,0,0,1],"type":"IDENTITY"},"tag":"NONE","velocity":{"x":0,"y":0}}
actual:
{"matrix":{"entries":[1,0,0,0,1,0,-0.74102534383174,0.03,1],"type":"TRANSLATION_2D"},"stepMatrix":{"entries":[1,0,0,0,1,0,0,0,1],"type":"IDENTITY"},"originalMatrix":{"entries":[1,0,0,0,1,0,-0.74102534383174,0.03,1],"type":"TRANSLATION_2D"},"canRotate":false,"canMove":false,"tag":"NONE","massShape":"BLOCK","position":{"x":-0.7410253524780274,"y":0.030000001192092896},"velocity":{"x":0,"y":0},"force":{"x":0,"y":0}}

Error: Assertion failed: Designed API changes detected, please roll them back or revise the reference API:
at window.assertions.assertFunction (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1681254360470/assert/js/assert.js:28:13)
at assert (phetioEngine.ts:365:22)
id: Bayes Puppeteer
Snapshot from 4/11/2023, 5:06:00 PM
@jonathanolson
Copy link
Contributor

This looks like #133 is not solved... the Mystery screen scale's initial position depends on the invisible barrier.

So we still have "initial data needs to be different based on the user's screen". @samreid thoughts?

@samreid
Copy link
Member

samreid commented Apr 19, 2023

@zepumph and I discussed this with @jonathanolson and we have a workaround for phetioAPICompare that will work ok for RC. For a longer-term solution, we may need to introduce a new PhetioObject metadata key that can be used like this:

Subject: [PATCH] Add UI sounds and projector mode, see https://github.com/phetsims/keplers-laws/issues/7
---
Index: js/phetioEngine.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/phetioEngine.ts b/js/phetioEngine.ts
--- a/js/phetioEngine.ts	(revision 6fe38d98385e545e436f95ee2669e96047865ca8)
+++ b/js/phetioEngine.ts	(date 1681924320674)
@@ -552,13 +552,15 @@
       level[ Tandem.METADATA_KEY ] = {} as PhetioObjectMetadata;
       const metadataObject = level[ Tandem.METADATA_KEY ];
 
-      const dataDefaults = phetioObject.phetioType.getAllDataDefaults();
-      const initialState = this.phetioStateEngine.getStateForObject( phetioID );
-      assert && assert( dataDefaults.hasOwnProperty( 'initialState' ), 'initialState expected to have a default on the IOType' );
-      if ( dataDefaults.initialState !== initialState && metadata.phetioState ) {
-        level[ Tandem.DATA_KEY ] = {
-          initialState: initialState
-        };
+      if ( metadata.phetioDesignedInitialState ) {
+        const dataDefaults = phetioObject.phetioType.getAllDataDefaults();
+        const initialState = this.phetioStateEngine.getStateForObject( phetioID );
+        assert && assert( dataDefaults.hasOwnProperty( 'initialState' ), 'initialState expected to have a default on the IOType' );
+        if ( dataDefaults.initialState !== initialState && metadata.phetioState ) {
+          level[ Tandem.DATA_KEY ] = {
+            initialState: initialState
+          };
+        }
       }
 
       Object.keys( metadata ).forEach( key => {

Adding a metadata key is not trivial, so I don't know if it should be done for this RC or not. Also, we try to minimize those keys since we have a lot of PhetioObjects and don't want too much memory burden.

jonathanolson added a commit to phetsims/chipper that referenced this issue Apr 19, 2023
@jonathanolson
Copy link
Contributor

Should be handled with the above workaround.

@jonathanolson jonathanolson removed their assignment Apr 19, 2023
zepumph added a commit to phetsims/chipper that referenced this issue Jul 6, 2023
@zepumph
Copy link
Member

zepumph commented Jul 6, 2023

This should be fixed generally by https://github.com/phetsims/phet-io/issues/1951

@zepumph
Copy link
Member

zepumph commented Jan 18, 2024

Seems like the solution here is to create an html page with the sim in an iframe with a constant size. This can be used for api-comparison AND for generation. This would make the screen size constant across all modes.

@zepumph zepumph self-assigned this Jan 18, 2024
@zepumph
Copy link
Member

zepumph commented Jan 22, 2024

We punted on this for now, because it wouldn't be a general enough fix.

@zepumph zepumph removed their assignment Jan 22, 2024
@zepumph zepumph self-assigned this Feb 12, 2024
@zepumph
Copy link
Member

zepumph commented Feb 12, 2024

I'd like to do some investigation about this for Buoyancy because we will have to add to this workaround. If we lock in api-generation browser dimensions like mentioned in phetsims/aqua#200 that may help remove this workaround.

@samreid samreid removed their assignment Aug 6, 2024
@samreid samreid added the dev:help-wanted Extra attention is needed label Aug 7, 2024
@samreid samreid removed the dev:help-wanted Extra attention is needed label Aug 14, 2024
@zepumph
Copy link
Member

zepumph commented Sep 9, 2024

I'm going to close this issue and we can work on a general solution over in https://github.com/phetsims/phet-io/issues/1951. @samreid and I made some good progress on how to keep "pure values" out of the API (and thus out of api comparison testing).

This CT error is closed, and a workaround will be in place until a general solution is done.

@zepumph zepumph closed this as completed Sep 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants