Add tap test for --extra-float-digits option
Hi hackers! I added tap test code for pg_dump --extra-float-digits option
because it hadn't tested it. There was no problem when writing test code
and running TAP tests.
Attachments:
0001-Add-tap-test-for-extra-float-digits-option.patchapplication/octet-stream; name=0001-Add-tap-test-for-extra-float-digits-option.patchDownload
From d13b12f12e1b07bd31e524d5041a891bf821b1af Mon Sep 17 00:00:00 2001
From: Dong Wook Lee <sh95119@gmail.com>
Date: Thu, 11 Jun 2020 13:45:02 +0900
Subject: [PATCH] Add tap test for --extra-float-digits option
I added this test for --extra-float-digits option
When an invalid range value is entered
---
src/bin/pg_dump/t/001_basic.pl | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/bin/pg_dump/t/001_basic.pl b/src/bin/pg_dump/t/001_basic.pl
index 550eab1ee3..b945e098d1 100644
--- a/src/bin/pg_dump/t/001_basic.pl
+++ b/src/bin/pg_dump/t/001_basic.pl
@@ -4,7 +4,7 @@ use warnings;
use Config;
use PostgresNode;
use TestLib;
-use Test::More tests => 78;
+use Test::More tests => 80;
my $tempdir = TestLib::tempdir;
my $tempdir_short = TestLib::tempdir_short;
@@ -175,3 +175,9 @@ command_fails_like(
qr/\Qpg_dumpall: error: option --exclude-database cannot be used together with -g\/--globals-only\E/,
'pg_dumpall: option --exclude-database cannot be used together with -g/--globals-only'
);
+
+command_fails_like(
+ [ 'pg_dump', '--extra-float-digits', '4' ],
+ qr/\Qpg_dump: error: extra_float_digits must be in range -15..3\E/,
+ 'pg_dump: extra_float_digits must be in range -15..3'
+);
--
2.24.3 (Apple Git-128)
On Thu, Jun 11, 2020 at 02:25:37PM +0900, Dong Wook Lee wrote:
Hi hackers! I added tap test code for pg_dump --extra-float-digits option
because it hadn't tested it. There was no problem when writing test code
and running TAP tests.
If we go down to that (there is a test for --compression), what about
--rows-per-insert?
--
Michael
Oh, now I understand. and I added a test of --row-per-insert option.
I'd better find more options missing test
2020년 6월 12일 (금) 오후 4:04, Michael Paquier <michael@paquier.xyz>님이 작성:
Show quoted text
On Fri, Jun 12, 2020 at 02:30:35PM +0900, Dong Wook Lee wrote:
Thank you for your response
Do you mean to move it under the test of --compression option?You could move the test where you see is best, and I would have done
that. My point is that we could have a test also for
--rows-per-insert as it deals with the same problem.
--
Michael
Attachments:
0002-pg_dump-Add-test-for-rows-per-insert-option.patchapplication/octet-stream; name=0002-pg_dump-Add-test-for-rows-per-insert-option.patchDownload
From ec8262b3599f6e802a2891fca9f968f7abdab569 Mon Sep 17 00:00:00 2001
From: Dong Wook Lee <sh95119@gmail.com>
Date: Thu, 11 Jun 2020 14:54:16 +0900
Subject: [PATCH 2/2] pg_dump: Add test for --rows-per-insert option
I add test of pg_dump --rows-per-insert option
this test check input range 1..2147483647
---
src/bin/pg_dump/t/001_basic.pl | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/bin/pg_dump/t/001_basic.pl b/src/bin/pg_dump/t/001_basic.pl
index b945e098d1..2ca18bc9c0 100644
--- a/src/bin/pg_dump/t/001_basic.pl
+++ b/src/bin/pg_dump/t/001_basic.pl
@@ -4,7 +4,7 @@ use warnings;
use Config;
use PostgresNode;
use TestLib;
-use Test::More tests => 80;
+use Test::More tests => 82;
my $tempdir = TestLib::tempdir;
my $tempdir_short = TestLib::tempdir_short;
@@ -181,3 +181,9 @@ command_fails_like(
qr/\Qpg_dump: error: extra_float_digits must be in range -15..3\E/,
'pg_dump: extra_float_digits must be in range -15..3'
);
+
+command_fails_like(
+ [ 'pg_dump', '--rows-per-insert', '-1' ],
+ qr/\Qpg_dump: error: rows-per-insert must be in range 1..2147483647\E/,
+ 'pg_dump: rows-per-insert must be in range 1..2147483647'
+);
--
2.24.3 (Apple Git-128)
Import Notes
Reply to msg id not found: 20200612070427.GC3362@paquier.xyz
On Fri, Jun 12, 2020 at 06:15:36PM +0900, Dong Wook Lee wrote:
Oh, now I understand. and I added a test of --row-per-insert option.
That's more of an habit to look around, find similar patterns and the
check if these are covered.
I have applied your patch, and you may want to be careful about a
couple of things:
- Please avoid top-posting on the mailing lists:
https://en.wikipedia.org/wiki/Posting_style#Top-posting
Top-posting breaks the logic of a thread.
- Your patch format is good. When sending a new version of the patch,
it may be better to send things as a complete diff on the master
branch (or the branch you are working on), instead of just sending one
patch that applies on top of something you sent previously. Here for
example your patch 0002 applied on top of 0001 that was sent at the
top of the thread. We have also guidelines about patch submission:
https://wiki.postgresql.org/wiki/Submitting_a_Patch
Thanks!
--
Michael
That's more of an habit to look around, find similar patterns and the
check if these are covered.I have applied your patch, and you may want to be careful about a
couple of things:
- Please avoid top-posting on the mailing lists:
https://en.wikipedia.org/wiki/Posting_style#Top-posting
Top-posting breaks the logic of a thread.
- Your patch format is good. When sending a new version of the patch,
it may be better to send things as a complete diff on the master
branch (or the branch you are working on), instead of just sending one
patch that applies on top of something you sent previously. Here for
example your patch 0002 applied on top of 0001 that was sent at the
top of the thread. We have also guidelines about patch submission:
https://wiki.postgresql.org/wiki/Submitting_a_PatchThanks!
--
Michael
Hi Michael
First of all, thank you for merging my patch.
And I'm sorry, I should have been more careful about it. Next time I
will follow format. And there is something I will tell you
Would you mind if I ask you specify my author info
with --author on the git commit?
The new contributor can get involved in the PostgreSQL project.
When they sent a patch and it was merged to the main repository,
it'd be better to keep the author info on the git commit, IMHO.
Because many opensource hackers who interested in
PostgreSQL project can want to keep a record of author info
on commits they wrote. Otherwise, contribution records can not be found
by 'git shortlog -sn' and GitHub and OpenHub cannot track their
opensource contribution records...
So what about using --author for PostgreSQL contributors
when merging their patches? like the Linux Kernel project
If so, many contributors would be highly encouraged.
Thanks,
Dong Wook
On Sat, Jun 13, 2020 at 06:34:46PM +0900, Dong Wook Lee wrote:
First of all, thank you for merging my patch.
And I'm sorry, I should have been more careful about it. Next time I
will follow format. And there is something I will tell you
We are all here to learn. It is good to begin with small
contributions to get a sense of how the project works, so I think that
you are doing well.
Because many opensource hackers who interested in
PostgreSQL project can want to keep a record of author info
on commits they wrote. Otherwise, contribution records can not be found
by 'git shortlog -sn' and GitHub and OpenHub cannot track their
opensource contribution records...So what about using --author for PostgreSQL contributors
when merging their patches? like the Linux Kernel project
That may be something to discuss with the project policy per-se. When
it comes to credit people, committers list authors, reviewers,
reporters, etc. directly in the commit log. And your name is
mentioned in 64725728, I made sure of it. The latest discussions we
had about the commit log format involved encouraging as much as
possible the use of a "Discussion" tag in commit logs, the rest
depends on each committer, and nobody uses --author.
--
Michael