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

Relax print tests #476

Merged
merged 4 commits into from
Nov 29, 2020
Merged

Relax print tests #476

merged 4 commits into from
Nov 29, 2020

Conversation

oxinabox
Copy link
Member

Wether or not a space is printed changes depending on the julia version.
So we use a regex.

I also hard coded to use Int32 since that simpifies the regex

@KristofferC
Copy link
Collaborator

KristofferC commented Nov 18, 2020

Why is this tested at all? Doesn't it just directly call a Base function?

If we really want to test it, I find it best to just interpolate the whole type, so something like:

@test ... == "3-element $(ForwardDiff.Partials{3,Int}):\n 1\n 2\n 3"

@oxinabox
Copy link
Member Author

Why is this tested at all? Doesn't it just directly call a Base function?

That is a good question.
We do have
https://github.com/JuliaDiff/ForwardDiff.jl/blob/master/src/partials.jl#L228
which itself looks kinda iffy.
Are we testing that that one isn't hit?

That code was added in
1279080
along with a ton of other things.

but that test was added in 799c444
Maybe @timholy can illuminate what we were testing for.

@andreasnoack
Copy link
Member

The show method for Partials used to fail which was why the test was added in #437. I've applied the proposal from #476 (comment).

@codecov-io
Copy link

codecov-io commented Nov 29, 2020

Codecov Report

Merging #476 (317859d) into master (3fb16ac) will increase coverage by 3.94%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #476      +/-   ##
==========================================
+ Coverage   84.92%   88.86%   +3.94%     
==========================================
  Files          10       10              
  Lines         703      826     +123     
==========================================
+ Hits          597      734     +137     
+ Misses        106       92      -14     
Impacted Files Coverage Δ
src/prelude.jl 87.50% <0.00%> (-5.10%) ⬇️
src/jacobian.jl 99.26% <0.00%> (-0.74%) ⬇️
src/hessian.jl 97.43% <0.00%> (+0.46%) ⬆️
src/gradient.jl 100.00% <0.00%> (+1.38%) ⬆️
src/apiutils.jl 100.00% <0.00%> (+2.63%) ⬆️
src/derivative.jl 100.00% <0.00%> (+3.70%) ⬆️
src/dual.jl 78.74% <0.00%> (+4.03%) ⬆️
src/partials.jl 84.21% <0.00%> (+7.19%) ⬆️
src/config.jl 87.03% <0.00%> (+19.64%) ⬆️

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 3fb16ac...317859d. Read the comment docs.

@andreasnoack andreasnoack merged commit c91eff4 into master Nov 29, 2020
@andreasnoack andreasnoack deleted the ox/strtests branch November 29, 2020 19:31
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.

None yet

4 participants