Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Add tests for lerpDouble #20778

Merged
merged 1 commit into from
Aug 26, 2020
Merged

Add tests for lerpDouble #20778

merged 1 commit into from
Aug 26, 2020

Conversation

cbracken
Copy link
Member

@cbracken cbracken commented Aug 26, 2020

Description

Add tests for lerpDouble.

The behaviour of lerpDouble with respect to null inputs isn't entirely obvious. In the case where both inputs are null, it returns null. Otherwise, it defaults the null parameter to 0.0 and carries on.

Post non-null by default, it might be nice to strengthen the parameter contract to require them to be non-null. While this would be a breaking change, it seems likely that the framework either meets this guarantee or can provide it without a framework breaking change.

Related Issues

flutter/flutter#64617 tracks making the parameters non-nullable.

Tests

This adds Dart unittests for lerpDouble.

The behaviour of lerpDouble with respect to null inputs isn't entirely
obvious. In the case where both inputs are null, it returns null.
Otherwise, it defaults the null parameter to 0.0 and carries on.

Post non-null by default, it might be nice to strengthen the parameter
contract to require them to be non-null. While this would be a breaking
change, it seems likely that the framework either meets this guarantee
or can provide it without a framework breaking change.

flutter/flutter#64617 tracks the above.

In the meantime, adding a test to lock in the current behaviour.
@cbracken
Copy link
Member Author

cbracken commented Aug 26, 2020

/cc @matthew-carroll as this relates to #20775.

/cc @yjbanov for thoughts on NNBD-related change. Discussion in flutter/flutter#64617.

@auto-assign auto-assign bot requested a review from GaryQian August 26, 2020 06:40
Copy link
Contributor

@matthew-carroll matthew-carroll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for adding these.

BTW, can you let me know how one executes these tests? This whole area of the engine looks like an unusual Dart setup...

@cbracken
Copy link
Member Author

cbracken commented Aug 26, 2020

Our documentation on running the Dart tests could definitely be better.

The general command for running all the tests from the engine/src/flutter directory is:

./testing/run_tests.sh

To run just the Dart tests, you can run the following command (which is one of the commands in the above script):

python ./testing/run_tests.py --variant=host_debug_unopt --type=dart

@Hixie
Copy link
Contributor

Hixie commented Aug 26, 2020

@cbracken please add that to the wiki!

expect(lerpDouble(0.0, 10.0, 5.0), 50.0);
expect(lerpDouble(10.0, 0.0, 5.0), -40.0);
});
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could also test infinities, NaNs, very large numbers, very small numbers...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call. I'll land this as-is for the moment and make the wiki updates so that @matthew-carroll is unblocked on his work, but will add those in a followup PR.

Copy link
Contributor

@Hixie Hixie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@cbracken
Copy link
Member Author

@cbracken please add that to the wiki!

Actually, turns out we did add it to the wiki at some point. Thanks @xster! This definitely used to be very under-documented, but the docs there are looking pretty comprehensive now.

https://github.com/flutter/flutter/wiki/Testing-the-engine#dart---dartui

@cbracken cbracken merged commit 388193a into flutter:master Aug 26, 2020
@cbracken cbracken deleted the lerp-test branch August 26, 2020 18:32
@xster
Copy link
Member

xster commented Aug 26, 2020

😲 @cbracken and @matthew-carroll join forces? Avengers assemble! 🦸‍♂️

@matthew-carroll
Copy link
Contributor

avengers

tvolkert pushed a commit to flutter/flutter that referenced this pull request Aug 27, 2020
* b08c6b9 Fixing synthesizing keys for multiple keys pressed down on flutter web (flutter/engine#19632)

* 8308b6a Avoid passing nil to IOS accessibility announcement  (flutter/engine#20700)

* 950b6a0 Roll Skia from ead4ca052b99 to 5da7327358e3 (1 revision) (flutter/engine#20782)

* 87fd0e4 Guard recording_canvas against restore calls without ending recording (flutter/engine#20786)

* 388193a Add tests for lerpDouble (flutter/engine#20778)
smadey pushed a commit to smadey/flutter that referenced this pull request Aug 27, 2020
* b08c6b9 Fixing synthesizing keys for multiple keys pressed down on flutter web (flutter/engine#19632)

* 8308b6a Avoid passing nil to IOS accessibility announcement  (flutter/engine#20700)

* 950b6a0 Roll Skia from ead4ca052b99 to 5da7327358e3 (1 revision) (flutter/engine#20782)

* 87fd0e4 Guard recording_canvas against restore calls without ending recording (flutter/engine#20786)

* 388193a Add tests for lerpDouble (flutter/engine#20778)
mingwandroid pushed a commit to mingwandroid/flutter that referenced this pull request Sep 6, 2020
* b08c6b9 Fixing synthesizing keys for multiple keys pressed down on flutter web (flutter/engine#19632)

* 8308b6a Avoid passing nil to IOS accessibility announcement  (flutter/engine#20700)

* 950b6a0 Roll Skia from ead4ca052b99 to 5da7327358e3 (1 revision) (flutter/engine#20782)

* 87fd0e4 Guard recording_canvas against restore calls without ending recording (flutter/engine#20786)

* 388193a Add tests for lerpDouble (flutter/engine#20778)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants