Skip to content

Systemize fixed point types to be tested #208

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

Merged
merged 4 commits into from
Aug 6, 2020

Conversation

kimikage
Copy link
Collaborator

@kimikage kimikage commented Aug 4, 2020

cf. #139 (comment)

This does not commonize the tests between Fixed and Normed, but does prepare for the commonization.

@codecov
Copy link

codecov bot commented Aug 4, 2020

Codecov Report

Merging #208 into master will decrease coverage by 0.62%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #208      +/-   ##
==========================================
- Coverage   89.83%   89.21%   -0.63%     
==========================================
  Files           6        6              
  Lines         482      482              
==========================================
- Hits          433      430       -3     
- Misses         49       52       +3     
Impacted Files Coverage Δ
src/fixed.jl 87.61% <0.00%> (-2.66%) ⬇️
src/normed.jl 89.30% <0.00%> (-1.02%) ⬇️
src/FixedPointNumbers.jl 89.47% <0.00%> (+1.41%) ⬆️

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 33f2583...837bc9c. Read the comment docs.

@timholy
Copy link
Member

timholy commented Aug 4, 2020

Due to code movement this won't be easy to review, and it doesn't look like it's much in need anyway. So just go ahead and merge when you're ready.

@kimikage
Copy link
Collaborator Author

kimikage commented Aug 5, 2020

Thank you for your response. Most of the code movement is not important, but I don't want to mess with the arrangement every time I add a test set. 😅

The essential change in this PR is 2a4a61b 8598955.
TBH, I don't really like the new target function because I think the test code should be simple. However, test case "justification" is important, and this change saves more than 15 seconds of testing time on my local machine.

@timholy
Copy link
Member

timholy commented Aug 5, 2020

Test performance is definitely important. Maybe add a little bit of commentary about the heavy/default/thin/light system? Just from reading it's not entirely obvious since the target_f_series is elsewhere.

@kimikage
Copy link
Collaborator Author

kimikage commented Aug 6, 2020

I moved the target_f_series to "test/common.jl". (it is contrary to the philosophy of specialization, though. 😄)
Also, I added the comment about the heavy/default/light/thin system as the docstring for target_f.

@timholy
Copy link
Member

timholy commented Aug 6, 2020

LGTM!

@kimikage kimikage merged commit 40c9117 into JuliaMath:master Aug 6, 2020
@kimikage kimikage deleted the reorder_testsets branch August 6, 2020 14:44
kimikage added a commit to kimikage/FixedPointNumbers.jl that referenced this pull request Apr 27, 2024
This abstracts the selection of the types to be tested and makes it more systematic.
As a result, we can reduce impractical test cases (e.g. for the 128-bit types) without significantly compromising the test quality.
This also reorders some test sets.
@kimikage kimikage mentioned this pull request Apr 30, 2024
38 tasks
kimikage added a commit to kimikage/FixedPointNumbers.jl that referenced this pull request Apr 30, 2024
This abstracts the selection of the types to be tested and makes it more systematic.
As a result, we can reduce impractical test cases (e.g. for the 128-bit types) without significantly compromising the test quality.
This also reorders some test sets.
kimikage added a commit to kimikage/FixedPointNumbers.jl that referenced this pull request May 1, 2024
This abstracts the selection of the types to be tested and makes it more systematic.
As a result, we can reduce impractical test cases (e.g. for the 128-bit types) without significantly compromising the test quality.
This also reorders some test sets.
kimikage added a commit to kimikage/FixedPointNumbers.jl that referenced this pull request May 1, 2024
This abstracts the selection of the types to be tested and makes it more systematic.
As a result, we can reduce impractical test cases (e.g. for the 128-bit types) without significantly compromising the test quality.
This also reorders some test sets.
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.

2 participants