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

Add a tool to facilitate comparative testing #87

Open
samreid opened this issue Mar 10, 2020 · 6 comments
Open

Add a tool to facilitate comparative testing #87

samreid opened this issue Mar 10, 2020 · 6 comments

Comments

@samreid
Copy link
Member

samreid commented Mar 10, 2020

From https://github.com/phetsims/phet-io-wrappers/issues/337

Inspired by phetsims/gravity-and-orbits#308, we would like to be able to test when a pure refactor has the same behavior as the previous version. Scenery has a similar feature, but if I recall correctly, it stores to disk and is non-interactive.

This version will run two versions from separate checkouts, and allow interactive or fuzzing.

@samreid samreid self-assigned this Mar 10, 2020
@samreid
Copy link
Member Author

samreid commented Mar 10, 2020

This patch allowed me to pixel compare two live simulations interactively:
https://github.com/phetsims/phet-io-wrappers/issues/337#issuecomment-597037791

There are some disadvantages, and I didn't commit.

  • Other source is hardcoded to port 8080, rest of the path required to be the same
  • Nontrivial changes to handlePlaybackEvent.
  • I tried to make it synchronous on every frame, but that was too demanding, so now it runs every Nth frame

Could we make it asynchronous, and just rely on the user pausing the simulation to compare?

Not sure how useful this would be.

@samreid
Copy link
Member Author

samreid commented Mar 10, 2020

It would be great to be able to run this on production versions and non-phet-io versions. I'll move this to aqua.

UPDATE: phet-io-wrappers is private and hence cannot move issues to a public repo. I copied things manually.

samreid added a commit to phetsims/sherpa that referenced this issue Mar 11, 2020
samreid added a commit that referenced this issue Mar 11, 2020
samreid added a commit that referenced this issue Mar 11, 2020
samreid added a commit that referenced this issue Mar 11, 2020
@samreid
Copy link
Member Author

samreid commented Mar 11, 2020

This is working well, I compared master of gravity and orbits to the published version. I'd like to run this past @jonathanolson to see if he has ideas or recommendations, then it would be nice to share with the dev team.

@samreid
Copy link
Member Author

samreid commented Mar 11, 2020

Questions to discuss with @jonathanolson:

  • How can we keep the clocks synchronized?
  • Can/should the QA team be able to use this tool? If so, how would they set up for it?
  • Any ideas for improvements?
  • How do you recommend to size the iframes and diff?
  • Where would you put query parameters specific to the wrapper?
  • Should there be a way to apply query parameters to both URLs? As a way of "factoring out" things like ?showPointers?
  • What should it be named?

samreid added a commit that referenced this issue Mar 11, 2020
@samreid
Copy link
Member Author

samreid commented Mar 11, 2020

Notes from discussion with @jonathanolson.

  • Use playbackMode=true, so it doesn't run its own step
  • use same seed
  • step frame like
function sendStep( dt ) {
  iframe.contentWindow.phet.joist.sim.stepSimulation( dt );
}
  • for future, could add postMessage pattern to make it work with different origins. But then we would lose synchronous.
  • Could add a "launcher" page where you put in the URLs. Doesn't need to be human readable.
  • Could display URLs in the page in textboxes.
  • For now, provide as a general tool. In the future we could leave it open to factor out query parameters, etc.
  • Rename to "visual-diff"
  • Could add an entry in aqua phetmarks if the team thinks it would be useful--and has a UI for setting URLs
  • See also snapshot-comparison.html which measures hashes for comparison.

@samreid
Copy link
Member Author

samreid commented Mar 26, 2020

I shared this with the team at status meeting a few weeks ago, so the team is roughly aware of it. I don't plan to work on it at the moment, but will add improvements as necessary as I used it in the future.

@samreid samreid removed their assignment Mar 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant