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

Adds tests for Type Stability of FD functions. #48

Merged
merged 2 commits into from
Feb 15, 2019

Conversation

NHDaly
Copy link
Member

@NHDaly NHDaly commented Feb 13, 2019

Test that most of the API functions for FixedDecimals are type stable in
their return type, using the @inferred test macro.

Tests this for all built-in integer types, across many precisions.

(Adds around 10 seconds to test time.)


My changes in PR #45 introduced a type instability I didn't know about. I fixed it and added unit tests for type stability of multiplication in these two commits, #45/files/beb1d3..11ee5e14, but I think that a nicer solution might just be to add type-stability tests across the whole API, so that these kinds of mistakes would be caught in the future. :)

This is a test-only change so hopefully is easy to review. :)
Cheers!

Test that most of the API functions for FixedDecimals are type stable in
their return type, using the `@inferred` test macro.

Tests this for all built-in integer types, across many precisions.

(Adds around 10 seconds to test time.)
@NHDaly NHDaly self-assigned this Feb 13, 2019
@codecov-io
Copy link

codecov-io commented Feb 13, 2019

Codecov Report

Merging #48 into master will increase coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #48      +/-   ##
==========================================
+ Coverage   96.49%   96.51%   +0.02%     
==========================================
  Files           1        1              
  Lines         171      172       +1     
==========================================
+ Hits          165      166       +1     
  Misses          6        6
Impacted Files Coverage Δ
src/FixedPointDecimals.jl 96.51% <0%> (+0.02%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 315e5cb...8cf3523. Read the comment docs.

test/runtests.jl Outdated Show resolved Hide resolved
test/runtests.jl Outdated Show resolved Hide resolved
@coveralls
Copy link

coveralls commented Feb 13, 2019

Coverage Status

Coverage decreased (-1.1%) to 95.349% when pulling 0b4c48c on NHDaly:type-stability-tests into 315e5cb on JuliaMath:master.

Make all the inferred tests actually test the return type is the correct
type by comparing `===` instead of `==`.
Make the unary test `@inferred(typemax(FD{T,f}))` test that the return
type is the correct type via `isa`.
@NHDaly
Copy link
Member Author

NHDaly commented Feb 13, 2019

Should be good now then. Thanks.

@omus omus merged commit c9c4686 into JuliaMath:master Feb 15, 2019
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

Successfully merging this pull request may close these issues.

4 participants