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
DirHandle: Improve test coverage and provide descriptions #15497
Comments
From @jkeenanThe coverage of DirHandle provided by tests in lib/DirHandle.t is poor and none of the individual tests have descriptions. The patch attached provides full statement, branch, condition and subroutine coverage and provides descriptions for all tests. Please review, particularly with respect to VMS and other edge cases. Thank you very much. |
From @jkeenan0001-Add-additional-tests-for-DirHandle-to-improve-covera.patchFrom af76633d9ddab4b0bb387be7ec3d0058a6db5a52 Mon Sep 17 00:00:00 2001
From: James E Keenan <jkeenan@cpan.org>
Date: Fri, 5 Aug 2016 19:13:15 -0400
Subject: [PATCH] Add additional tests for DirHandle to improve coverage.
Add descriptions to all tests.
---
lib/DirHandle.t | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 87 insertions(+), 8 deletions(-)
diff --git a/lib/DirHandle.t b/lib/DirHandle.t
index f3a9304..6e3eb01 100644
--- a/lib/DirHandle.t
+++ b/lib/DirHandle.t
@@ -11,7 +11,7 @@ BEGIN {
}
use DirHandle;
-use Test::More tests => 5;
+use Test::More tests => 28;
# Fetching the list of files in two different ways and expecting them
# to be the same is a race condition when tests are running in parallel.
@@ -25,22 +25,101 @@ if ($ENV{PERL_CORE} && -d 'uni') {
$dot = DirHandle->new('.');
-is(defined $dot, 1);
+ok(defined $dot, "DirHandle->new returns defined value");
+isa_ok($dot, 'DirHandle');
@a = sort <*>;
do { $first = $dot->read } while defined($first) && $first =~ /^\./;
-ok(+(grep { $_ eq $first } @a));
+ok(+(grep { $_ eq $first } @a),
+ "Scalar context: First non-dot entry returned by 'read' is found in glob");
@b = sort($first, (grep {/^[^.]/} $dot->read));
-ok(+(join("\0", @a) eq join("\0", @b)));
+ok(+(join("\0", @a) eq join("\0", @b)),
+ "List context: Remaining entries returned by 'read' match glob");
-$dot->rewind;
+ok($dot->rewind, "'rewind' method returns true value");
@c = sort grep {/^[^.]/} $dot->read;
-cmp_ok(join("\0", @b), 'eq', join("\0", @c));
+cmp_ok(join("\0", @b), 'eq', join("\0", @c),
+ "After 'rewind', directory re-read as expected");
-$dot->close;
+ok($dot->close, "'close' method returns true value");
$dot->rewind;
-is(defined $dot->read, '');
+ok(! defined $dot->read,
+ "Having closed the directory handle -- and notwithstanding invocation of 'rewind' -- 'read' returns undefined value");
+
+{
+ local $@;
+ eval { $redot = DirHandle->new( '.', '..' ); };
+ like($@, qr/^usage/,
+ "DirHandle constructor with too many arguments fails as expected");
+}
+
+# Now let's test with directory argument provided to 'open' rather than 'new'
+
+$redot = DirHandle->new();
+ok(defined $redot, "DirHandle->new returns defined value even without provided argument");
+isa_ok($redot, 'DirHandle');
+ok($redot->open('.'), "Explicit call of 'open' method returns true value");
+do { $first = $redot->read } while defined($first) && $first =~ /^\./;
+ok(+(grep { $_ eq $first } @a),
+ "Scalar context: First non-dot entry returned by 'read' is found in glob");
+
+@b = sort($first, (grep {/^[^.]/} $redot->read));
+ok(+(join("\0", @a) eq join("\0", @b)),
+ "List context: Remaining entries returned by 'read' match glob");
+
+ok($redot->rewind, "'rewind' method returns true value");
+@c = sort grep {/^[^.]/} $redot->read;
+cmp_ok(join("\0", @b), 'eq', join("\0", @c),
+ "After 'rewind', directory re-read as expected");
+
+ok($redot->close, "'close' method returns true value");
+$redot->rewind;
+ok(! defined $redot->read,
+ "Having closed the directory handle -- and notwithstanding invocation of 'rewind' -- 'read' returns undefined value");
+
+$undot = DirHandle->new('foobar');
+ok(! defined $undot,
+ "Constructor called with non-existent directory returns undefined value");
+
+# Test error conditions for various methods
+
+$aadot = DirHandle->new();
+ok(defined $aadot, "DirHandle->new returns defined value even without provided argument");
+isa_ok($aadot, 'DirHandle');
+{
+ local $@;
+ eval { $aadot->open('.', '..'); };
+ like($@, qr/^usage/,
+ "'open' called with too many arguments fails as expected");
+}
+ok($aadot->open('.'), "Explicit call of 'open' method returns true value");
+{
+ local $@;
+ eval { $aadot->read('foobar'); };
+ like($@, qr/^usage/,
+ "'read' called with argument fails as expected");
+}
+{
+ local $@;
+ eval { $aadot->close('foobar'); };
+ like($@, qr/^usage/,
+ "'close' called with argument fails as expected");
+}
+{
+ local $@;
+ eval { $aadot->rewind('foobar'); };
+ like($@, qr/^usage/,
+ "'rewind' called with argument fails as expected");
+}
+ok($aadot->DESTROY, "'DESTROY' method returns true value");
+
+{
+ local $@;
+ eval { $bbdot = DirHandle::new(); };
+ like($@, qr/^usage/,
+ "DirHandle called as function but with no arguments fails as expected");
+}
if ($chdir) {
chdir "..";
--
2.7.4
|
From @tonycozOn Fri Aug 05 16:17:41 2016, jkeenan wrote:
+ok($aadot->DESTROY, "'DESTROY' method returns true value"); We don't want to care about the return value of the DESTROY method. You don't explicitly test $dh->open("unknown"), nor reading from, Tony |
The RT System itself - Status changed from 'new' to 'open' |
From @jkeenanOn Sun Aug 14 19:07:47 2016, tonyc wrote:
Can you suggest another, better way to exercise the DESTROY method? (I added this test when I saw through coverage analysis that I was not previously exercising it.)
-- |
From @jkeenanOn Sun Aug 14 19:07:47 2016, tonyc wrote:
Attaching second patch with tests for these cases. -- |
From @jkeenan0002-Add-tests-starting-with-open-on-nonexistent-director.patchFrom 758135d8aa4e98efa0b2a4738dde6849a43d0a32 Mon Sep 17 00:00:00 2001
From: James E Keenan <jkeenan@cpan.org>
Date: Mon, 15 Aug 2016 08:44:30 -0400
Subject: [PATCH] Add tests starting with 'open' on nonexistent directory.
Per suggestion by Tony Cook.
For: RT #128856
---
lib/DirHandle.t | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/lib/DirHandle.t b/lib/DirHandle.t
index 6e3eb01..03b71b4 100644
--- a/lib/DirHandle.t
+++ b/lib/DirHandle.t
@@ -11,7 +11,7 @@ BEGIN {
}
use DirHandle;
-use Test::More tests => 28;
+use Test::More tests => 32;
# Fetching the list of files in two different ways and expecting them
# to be the same is a race condition when tests are running in parallel.
@@ -121,6 +121,16 @@ ok($aadot->DESTROY, "'DESTROY' method returns true value");
"DirHandle called as function but with no arguments fails as expected");
}
+$bbdot = DirHandle->new();
+ok(! $bbdot->open('foobar'),
+ "Calling open method on nonexistent directory returns false value");
+ok(! $bbdot->read(),
+ "Calling read method after failed open method returns faluse value");
+ok(! $bbdot->rewind(),
+ "Calling rewind method after failed open method returns faluse value");
+ok(! $bbdot->close(),
+ "Calling close method after failed open method returns faluse value");
+
if ($chdir) {
chdir "..";
}
--
2.7.4
|
From @jkeenanOn Mon Aug 15 05:47:57 2016, jkeenan wrote:
Tony C: Does the second patch address your concerns? Thank you very much. -- |
From @jkeenanOn Mon Aug 22 06:32:17 2016, jkeenan wrote:
Tony C: Ping re second patch. Thank you very much. -- |
From @tonycozOn Tue Aug 30 09:22:45 2016, jkeenan wrote:
There's some spelling errors in the test comments: + "Calling read method after failed open method returns faluse value"); repeated in 2 other places. As to DESTROY, all you can really do to test it is to check for resource Testing the return value of DESTROY doesn't really check that any resources Tony |
From @jkeenanOn Mon Sep 05 05:07:59 2016, tonyc wrote:
Please review replacement patch attached. Thank you very much. |
From @jkeenan128856-0001-Add-additional-tests-for-DirHandle-to-improve-covera.patchFrom 37e2ceb8bb60f11112e271d07b5bd31c9b1670d6 Mon Sep 17 00:00:00 2001
From: James E Keenan <jkeenan@cpan.org>
Date: Fri, 5 Aug 2016 19:13:15 -0400
Subject: [PATCH] Add additional tests for DirHandle to improve coverage.
Add descriptions to all tests. Revise per suggestions by Tony Cook.
For: RT #128856
---
lib/DirHandle.t | 104 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 96 insertions(+), 8 deletions(-)
diff --git a/lib/DirHandle.t b/lib/DirHandle.t
index f3a9304..2a131e6 100644
--- a/lib/DirHandle.t
+++ b/lib/DirHandle.t
@@ -11,7 +11,7 @@ BEGIN {
}
use DirHandle;
-use Test::More tests => 5;
+use Test::More tests => 31;
# Fetching the list of files in two different ways and expecting them
# to be the same is a race condition when tests are running in parallel.
@@ -25,22 +25,110 @@ if ($ENV{PERL_CORE} && -d 'uni') {
$dot = DirHandle->new('.');
-is(defined $dot, 1);
+ok(defined $dot, "DirHandle->new returns defined value");
+isa_ok($dot, 'DirHandle');
@a = sort <*>;
do { $first = $dot->read } while defined($first) && $first =~ /^\./;
-ok(+(grep { $_ eq $first } @a));
+ok(+(grep { $_ eq $first } @a),
+ "Scalar context: First non-dot entry returned by 'read' is found in glob");
@b = sort($first, (grep {/^[^.]/} $dot->read));
-ok(+(join("\0", @a) eq join("\0", @b)));
+ok(+(join("\0", @a) eq join("\0", @b)),
+ "List context: Remaining entries returned by 'read' match glob");
-$dot->rewind;
+ok($dot->rewind, "'rewind' method returns true value");
@c = sort grep {/^[^.]/} $dot->read;
-cmp_ok(join("\0", @b), 'eq', join("\0", @c));
+cmp_ok(join("\0", @b), 'eq', join("\0", @c),
+ "After 'rewind', directory re-read as expected");
-$dot->close;
+ok($dot->close, "'close' method returns true value");
$dot->rewind;
-is(defined $dot->read, '');
+ok(! defined $dot->read,
+ "Having closed the directory handle -- and notwithstanding invocation of 'rewind' -- 'read' returns undefined value");
+
+{
+ local $@;
+ eval { $redot = DirHandle->new( '.', '..' ); };
+ like($@, qr/^usage/,
+ "DirHandle constructor with too many arguments fails as expected");
+}
+
+# Now let's test with directory argument provided to 'open' rather than 'new'
+
+$redot = DirHandle->new();
+ok(defined $redot, "DirHandle->new returns defined value even without provided argument");
+isa_ok($redot, 'DirHandle');
+ok($redot->open('.'), "Explicit call of 'open' method returns true value");
+do { $first = $redot->read } while defined($first) && $first =~ /^\./;
+ok(+(grep { $_ eq $first } @a),
+ "Scalar context: First non-dot entry returned by 'read' is found in glob");
+
+@b = sort($first, (grep {/^[^.]/} $redot->read));
+ok(+(join("\0", @a) eq join("\0", @b)),
+ "List context: Remaining entries returned by 'read' match glob");
+
+ok($redot->rewind, "'rewind' method returns true value");
+@c = sort grep {/^[^.]/} $redot->read;
+cmp_ok(join("\0", @b), 'eq', join("\0", @c),
+ "After 'rewind', directory re-read as expected");
+
+ok($redot->close, "'close' method returns true value");
+$redot->rewind;
+ok(! defined $redot->read,
+ "Having closed the directory handle -- and notwithstanding invocation of 'rewind' -- 'read' returns undefined value");
+
+$undot = DirHandle->new('foobar');
+ok(! defined $undot,
+ "Constructor called with non-existent directory returns undefined value");
+
+# Test error conditions for various methods
+
+$aadot = DirHandle->new();
+ok(defined $aadot, "DirHandle->new returns defined value even without provided argument");
+isa_ok($aadot, 'DirHandle');
+{
+ local $@;
+ eval { $aadot->open('.', '..'); };
+ like($@, qr/^usage/,
+ "'open' called with too many arguments fails as expected");
+}
+ok($aadot->open('.'), "Explicit call of 'open' method returns true value");
+{
+ local $@;
+ eval { $aadot->read('foobar'); };
+ like($@, qr/^usage/,
+ "'read' called with argument fails as expected");
+}
+{
+ local $@;
+ eval { $aadot->close('foobar'); };
+ like($@, qr/^usage/,
+ "'close' called with argument fails as expected");
+}
+{
+ local $@;
+ eval { $aadot->rewind('foobar'); };
+ like($@, qr/^usage/,
+ "'rewind' called with argument fails as expected");
+}
+
+{
+ local $@;
+ eval { $bbdot = DirHandle::new(); };
+ like($@, qr/^usage/,
+ "DirHandle called as function but with no arguments fails as expected");
+}
+
+$bbdot = DirHandle->new();
+ok(! $bbdot->open('foobar'),
+ "Calling open method on nonexistent directory returns false value");
+ok(! $bbdot->read(),
+ "Calling read method after failed open method returns false value");
+ok(! $bbdot->rewind(),
+ "Calling rewind method after failed open method returns false value");
+ok(! $bbdot->close(),
+ "Calling close method after failed open method returns false value");
if ($chdir) {
chdir "..";
--
2.7.4
|
@jkeenan - Status changed from 'open' to 'resolved' |
Migrated from rt.perl.org#128856 (status was 'resolved')
Searchable as RT128856$
The text was updated successfully, but these errors were encountered: