-
Notifications
You must be signed in to change notification settings - Fork 7
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
phetioCompareAPI is failing. #301
Comments
I'm also wondering why this error is not showing up in CT. I added |
I'll take a look! |
I pulled all, and then ran: Then I parsed the horrible json-ish output by replacing
All of the above look to be values of PhET-iO element's initial state that are based on a seeded random, and so are different between runtimes. This seems like something to try to manage, but I'm not sure off the top of my head the best solution. I'll create a general issue. |
Any idea why this is not showing up as an error in CT? |
After the above formatting commits, this is what the assertion message now looks like off the bat:
|
I found https://github.com/phetsims/phet-io/issues/1765, noting that the generated API file is consistent as it pertains to randomness. From this I don't feel like this needs to block publication. The issue is in our ability to maintain master and prevent changes from occurring that change the API on master. |
OK, I think I understand why this doesn't need to be blocking -- it's relevant to master, not a release branch. But if these types of errors persist, how will we identify real errors? And will these types of errors occur in CT? |
This is certainly something to fix, and to not let linger.
I poked around and tried to see if CT was seeding anything, purposefully or accidentally. I couldn't find anything. More investigation to be done in https://github.com/phetsims/phet-io/issues/1799 |
I recommend closing this issue after @pixelzoom notes that locally this link will not fail: And changing the randomSeed to anything else will cause an assertion error. We will continue the CT trouble over in https://github.com/phetsims/phet-io/issues/1799. |
I can use any number for You've also duplicated the API-generation seed "12345" in two places: generatePhetioMacroAPI.js, listContinuousTests.js. This should be factored out, not duplicated in string literals. Back to @zepumph. |
I see that I am pushed, pulled, and entirely on master.
Yes, it is duplicated in more than just those two spots. I cannot think of a place to factor it out to, see https://github.com/phetsims/phet-io/issues/1798#issuecomment-877845936 for discussion. Conclusion still TBD. |
Hmm.... It seems to be working as expected now. Maybe I didn't have something pulled, or had a cacheing problem. Sorry for the reassignment. Closing - because this pertains to master, it does not need to be verified by QA for phetsims/qa#662. |
For phetsims/qa#662
In #294, I followed all of the steps in https://github.com/phetsims/phet-io/blob/master/doc/phet-io-instrumentation-technical-guide.md#post-publication-steps for setting up
phetioDesigned: true
. Everything is commited and pushed.When I run NS (master) in Studio with
phetioCompareAPI
, it's failing with:The json part of the console output is way too long to be examined manually.
I've tried re-running
grunt generate-phet-io-api
, and no changes are generated, so there's nothing for me to diff.@samreid @zepumph suggestions on how to proceed?
The text was updated successfully, but these errors were encountered: