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

Test cleanup 3 #109

Merged
merged 65 commits into from Sep 9, 2016
Merged

Test cleanup 3 #109

merged 65 commits into from Sep 9, 2016

Conversation

zmughal
Copy link
Member

@zmughal zmughal commented May 24, 2015

Resolves #8, #9.

  • using Test::More module (no hand-rolled TAP or Test module)

  • use strict; use warnings everywhere

  • remove use of $a, $b lexicals (reserved for sort function)

  • using Test::Exception (lives_ok, dies_ok, etc.) rather than the eval { $code }; ok $@ combination

  • make each individual test run in its own scope
    e.g.

    {
      my $test = ...
      ok( ... );
    }
    {
      my $test = ...
      ok( ... );
    }

    This makes tests not dependent on previous code which means that they can be
    moved around without worrying about state. In the future, these should be setup
    as labelled subtests.

  • Using all approx( ... ) instead of a custom tapprox( ... ) in each file.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 62.4% when pulling 4d23291 on test-cleanup-3 into 18d5c33 on master.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling ef5bed4 on test-cleanup-3 into * on master*.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 08f129c on test-cleanup-3 into * on master*.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling e5b309c on test-cleanup-3 into * on master*.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling e5b309c on test-cleanup-3 into * on master*.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 62.4% when pulling 426ae51 on test-cleanup-3 into a3961b0 on master.

@mohawk2
Copy link
Member

mohawk2 commented May 30, 2015

I've taken the liberty of squashing together the commits that had the same label ie were on the same file.


ok(1);
done_testing;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You've got both tests => 1, and done_testing. Pick one?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, does having both help in case the test segfaults? I think you're right, I should remove it.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 62.43% when pulling bfa94de on test-cleanup-3 into 673f996 on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 62.43% when pulling 04e74e5 on test-cleanup-3 into 8e9bc6a on master.

@zmughal zmughal force-pushed the test-cleanup-3 branch 2 times, most recently from 0bbaf96 to 8733e16 Compare June 3, 2015 20:34
@zmughal zmughal modified the milestones: PDL v2.011, PDL v2.009 Jun 3, 2015
@zmughal
Copy link
Member Author

zmughal commented Jun 3, 2015

@coveralls
Copy link

Coverage Status

Coverage remained the same at 62.983% when pulling ccf1eba on test-cleanup-3 into 9640c2e on master.

@zmughal
Copy link
Member Author

zmughal commented Sep 2, 2016

TODO:

  • diff the test outputs wrt master to make sure no messages were lost
  • run Perl::Critic on the tests This will require another look during another refactoring pass.
  • rerun checks on what was listed in the top

@d-lamb
Copy link
Member

d-lamb commented Sep 3, 2016

Looks good to me.

@zmughal
Copy link
Member Author

zmughal commented Sep 3, 2016

I'll finish the last couple TODOs this weekend just to make sure everything checks out and then do the long-awaited merge!

@coveralls
Copy link

Coverage Status

Coverage remained the same at 62.983% when pulling 483140a on test-cleanup-3 into 9640c2e on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 62.983% when pulling f46696c on test-cleanup-3 into 9640c2e on master.

@zmughal
Copy link
Member Author

zmughal commented Sep 9, 2016

After doing a line-by-line comparison of the output, this is now ready to merge.

@zmughal zmughal merged commit f46696c into master Sep 9, 2016
@zmughal zmughal mentioned this pull request Sep 9, 2016
22 tasks
@zmughal zmughal deleted the test-cleanup-3 branch September 9, 2016 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor to use Test::More
4 participants