Added tests for issue #53, followup to PR #57. #65
Conversation
|
It is probably cleaner to append these changes to PR #57? |
Might help with CI failures.
Might help with CI failures.
… that indicates success for GDAL
* succesfully -> successfully
Merge branch 'fix_issue_53' into test_transform_53
|
Thank you for combining the commits. It seems, though, that something has happened to the commit authors of several commits in the merge process. It might be necessary to rebase your commits onto the master branch and then force push the PR. If it all gets too messy then a new PR might be simpler. And if that's too complex (as rebasing can be) then I can also cherry-pick your commits onto a new PR. |
|
Although the authors seem correct when pulling the changes locally so perhaps don't worry about the rebase. |
| @@ -210,6 +210,11 @@ if(1){ | |||
| is($p, $ogc_wkt, "Set/get projection string"); | |||
| my $transform = [10,2,0,20,0,3]; | |||
| $ds->SetGeoTransform($transform); | |||
| my $inv = [0,0,0,0,0,0]; | |||
| ok(Geo::GDAL::FFI::GDALInvGeoTransform($transform, $inv) && "@$inv" eq "-5 0.5 0 -6.66666666666667 0 0.333333333333333", "Invert geotransform"); | |||
There was a problem hiding this comment.
Can this be split into two tests? Store the result of the transform call into a scalar variable. It can be checked using the is function. Then use an is_deeply test to check the expected array values.
| my @y = ($ul[1], $lr[1], $ur[1], $ll[1]); | ||
| my $z = undef; | ||
| ok(Geo::GDAL::FFI::OCTTransform($ct, 4, \@x, \@y, \@$z), "Coordinate transformation 3D worked"); | ||
| ok(qq/@x @y/ eq $result, "Resulting coordinates"); |
There was a problem hiding this comment.
This is also better done using is_deeply.
There was a problem hiding this comment.
Same for similar cases below.
| ok(qq/@x @y/ eq $result, "Resulting coordinates"); | ||
| @ps = (0,0,0,0); | ||
| ok(Geo::GDAL::FFI::OCTTransform4D($ct, 4, \@x, \@y, \@$z, \@$t, \@ps), "Coordinate transformation 4D worked"); | ||
| ok(qq/@x @y/ eq $result && qq/@ps/ eq qq/1 1 1 1/, "Resulting coordinates"); |
There was a problem hiding this comment.
This is better written using is_deeply.
my $exp_results = [...]; # set up expected values
is_deeply ([@x, @y], $exp_result, "Resulting coordinates match");
is_deeply (\@ps, [qw/1 1 1 1/], "Resulting coordinates match, (ps array)");
The is and is_deeply type tests give better feedback when a test fails, i.e they say what was found and was was expected. ok just says there was a failure.
|
@fpl - I added some comments a while ago but it seems one has to publish them so they are visible to others. I have just done that so now you should hopefully be able to see them. If you don't have time then I can clean them up in a later commit. |
Yep, I can change the test code as suggested. |
Thanks @fpl. If you can rebase the changes on latest master then that would be useful. If it causes problems then don't worry about it. |
|
I'm closing because this is obsoleted by PR #68 |
Hi,
Here I added a few tests for issue #53, still missing a test for
OCTTransformBoundsbecause I honestly did not understand the use and context of that API function. Without PR 57, those tests should cause a failure due to wrong type of arguments (double[] instead of double*) i.e. argument type not a reference to scalar..
If you try to force the use of double *, that should cause a segfault in the Perl interpreter.
Cheers
PS: note that this PR does not include the fix proposed in #57, so the obvious failures indicated above are expected and present in the CI failures.