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

Convert a bunch of tests to Test::More, etc. #195

Closed
wants to merge 24 commits into from
Closed

Conversation

d-lamb
Copy link
Member

@d-lamb d-lamb commented Aug 29, 2016

No description provided.

The original regexp took 'unsigned short *' and captured only the
'short'. This changed allows it to capture 'unsigned short'.
Since fitgauss1d[r] has GenericTypes=>[D], it is safe to assume
that everything will be Double, so using fabs is preferred.
The switch statement was written long before those datatypes were
in PDL, and the compiler was nicely complaining about their absense.
Perl's 'warn' function had been undef'd because of problems with the gsl code's
'warn' variable. But PDL::PP inserts code into XS files that calls
Perl's 'warn' function.  To avoid this I removed the #undef warn and
changed the gsl variable name from warn to gslwarn.
Some function prototypes that get generated into SlatecProtos.h
have return types float or double, but most were just missing, and
the compiler assumed them to be int. The function defs all do
return int, so I just made it explicit.
A few functions (SDstart ... SDend) did not have function prototypes
in hdf.h, this other header file was necessary.
The list is generated by Types.pm.PL, but gets put into pdl.h and
pdlsimple.h. This was motivated by code in Bad.c that assigns to
a variable of type pdl_datatypes the value -1 (in some error condition
that is unlikely to be reached). This jives with the ANYVAL_FROM_CTYPE
macro in pdl.h.  Thanks to Chris Marshall and David Mertens for the
pointers.  See pdl-devel 2016-05-05 discussion.
This was motivated because we should have been using labs or llabs,
the correct function to call was system-dependent. This avoides all
that by replacing the abs() call with multiplication by the
variable's sign.
fabs is fine for doubles, but not for anything else: this inserts
the correct function (abs, labs, llabs, etc) based on the datatype
used in the complex division. I hope.
This silences a compiler warning about a printf wanting double ("%f")
but we passed an Anyval.
Most prototypes get inserted into SlatecProtos.h by defslatec.
But polfit_ and dpolft_ do not, so they need to be added manually.
@d-lamb
Copy link
Member Author

d-lamb commented Aug 30, 2016

@zmughal can you kick off the Travis CI builds again? The fact that the first one took 13+ minutes makes me suspicious. Or is there a way to get into the instances of those failing tests and manually run the tests to get some additional diagnostics?

@zmughal
Copy link
Member

zmughal commented Aug 30, 2016

I've restarted the failing builds again. You can do the same if you log in to Travis CI with your GitHub account.

From the logs, I see that one of the builds had a failure in t/image2d.t (not all the tests ran?) and t/lvalue.t (seemed to exit early?). We'll see if that was a fluke after the builds finish running.

By the way, I had some similar changes in #109 which hasn't been merged in yet. I can rebase it off master. It'll be a bit tricky, but that hasn't stopped me before! :-)

@mohawk2
Copy link
Member

mohawk2 commented Aug 30, 2016

Can I suggest we get both sets of changes rebased, deconflicted and merged? It hurts me to see such good work sitting there on a branch :-)

@d-lamb
Copy link
Member Author

d-lamb commented Aug 30, 2016

I'm able to reproduce the t/image2d.t failure on my machine building with no bad value support. Will investigate. I have no idea why t/lut.t would be exiting early.

I completely missed #109 when I started doing this, sorry. Let's get these both into master!

@d-lamb
Copy link
Member Author

d-lamb commented Aug 30, 2016

thanks for the pointers on how to kick off the builds again. I saw the "login with your Github account" button, but then when Travis wanted my personal information and my repositories and my first born I backed away quickly.

@d-lamb
Copy link
Member Author

d-lamb commented Aug 30, 2016

For t/lvalue.t, when running under Devel::Cover on my machine, in the BEGIN block the $PERLDB variable is 260, which causes it to drop into the else part of that block, which I didn't fix up properly. For reference, 260 = 0x104, which means: 0x100: Provide informative "file" names for evals based on the place they were compiled and 0x004: Switch off optimizations.

In that case, $PERLDB==260==0x104, which means:
0x100: Provide informative "file" names for evals based on the place they were compiled
0x004: Switch off optimizations
@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 62.983% when pulling dffd8c8 on test_more-ify into 0cce286 on master.

@zmughal
Copy link
Member

zmughal commented Aug 31, 2016

This looks good to go.

Merge: Aye.

Also, I just figured out the reason why I hadn't merged #109. I had merged it in pdla as PDLPorters/pdla-rest#7 instead.

@wchristian wchristian closed this Aug 31, 2016
@wchristian wchristian deleted the test_more-ify branch August 31, 2016 12:36
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

5 participants