Repository navigation
Improved testing and comparison tooling - #794
Merged
Merged
Conversation
|
Can't wait for this, this looks like a great tool. Thanks for all your work, @liabru . |
Merged
Merged
Merged
Merged
Merged
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I've been working on new tools to help review the result of any changes made to the codebase.
The new comparison tool is essentially an integration test which runs all of the examples using the current source and compares results against the latest release build. It produces a report flagging any:
Intrinsic properties generally shouldn't change (other than new properties) as this would indicate a likely regression, or at least a breaking change. For example, the
massof the same body should never change between versions.Extrinsic properties may change as they may have multiple acceptable values due to limited simulation accuracy. They indicate a change to the simulation behaviour, which may be acceptable and intended or may not be. For example, the
positionof the same body may change between versions if an accuracy improvement was since implemented. Whereas a code refactor for example, should not normally change the resulting extrinsic properties between versions.It's possible for example that a performance refactor might come with an accuracy trade-off that changes extrinsic properties whilst producing acceptable results. Generally extrinsic changes will require some further manual testing and review before deeming acceptance into the build (the visual comparison tool below can help with judging the difference).
Here's how to run the comparison integration test (included as part of the test suite):
npm run testornpm run test-watchThere is also a new browser-based visual comparison tool that runs both versions side by side and overlays both interactive simulations in real-time for comparing extrinsic changes. Below you can see how the catapult example behaviour quickly diverges from release version when the default friction is changed.
open http://localhost:8000/?compare=120#catapultTo visualise intrinsic changes, the tool can output them into two separate files for each version which can then be diff'd e.g. using vscode.
npm run test-savecode -n -d test/__compare__/examples-build.json test/__compare__/examples-dev.jsonThis approach is still experimental, so it will take some time to see how useful it is in practice. I might also add code size comparison too. If anybody has ideas on other improvements for this, feel free to post them here. Thanks.