-
Notifications
You must be signed in to change notification settings - Fork 100
PERL-805 Implement database enumeration spec #146
Conversation
xdg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty good. Just a couple minor changes, please.
| my @dbs = $client->database_names; | ||
| List of all database names on the MongoDB server. Supports filters in the same | ||
| way as L</"list_databases">. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add an example here as well for clarity.
t/enumerate_databases.t
Outdated
| } | ||
| my @all_dbs = $conn->list_databases; | ||
|
|
||
| ok( scalar( @all_dbs ) > 6, "Found at least 6 databases" ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
>= 6 here? Usually there would be 'admin' and 'test', but let's not make that assumption.
t/enumerate_databases.t
Outdated
|
|
||
| for my $prefix ( qw/ foo bar baz / ) { | ||
| my $db1 = $conn->get_database( $prefix . $time_prefix . int(rand(99999)) ) or die "Can't get database\n"; | ||
| my $db2 = $conn->get_database( $prefix . $time_prefix . int(rand(99999)) ) or die "Can't get database\n"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of getting databases this way, how about having get_test_db take an optional prefix (defaulting to 'testdb') and using that? That would have the added advantage of registering the databases for cleanup in the END step, which would mean you can remove lines 74-76.
t/enumerate_databases.t
Outdated
| my $time_prefix = time(); | ||
|
|
||
| for my $prefix ( qw/ foo bar baz / ) { | ||
| my $db1 = $conn->get_database( $prefix . $time_prefix . int(rand(99999)) ) or die "Can't get database\n"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto comments above
|
Squashed and merged. |
No description provided.