public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH 4/7] vcregress: add alltaptests
2+ messages / 2 participants
[nested] [flat]

* [PATCH 4/7] vcregress: add alltaptests
@ 2022-02-25 23:00 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 2+ messages in thread

From: Justin Pryzby @ 2022-02-25 23:00 UTC (permalink / raw)

In passing, document taptest PROVE_FLAGS

ci-os-only: windows

See also:
d835dd6685246f0737ca42ab68242210681bb220
13d856e177e69083f543d6383eeda9e12ce3c55c
fed6df486dca1b9e53d3f560031b9a236c99f4bb
---
 .cirrus.yml                 | 10 ++-------
 src/tools/msvc/vcregress.pl | 43 +++++++++++++++++++++++++++++++++----
 2 files changed, 41 insertions(+), 12 deletions(-)

diff --git a/.cirrus.yml b/.cirrus.yml
index 634c0b4933..bc4ae9fc28 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -417,14 +417,8 @@ task:
   test_ssl_script: |
     set with_ssl=openssl
     %T_C% perl src/tools/msvc/vcregress.pl taptest ./src/test/ssl/
-  test_subscription_script: |
-    %T_C% perl src/tools/msvc/vcregress.pl taptest ./src/test/subscription/
-  test_authentication_script: |
-    %T_C% perl src/tools/msvc/vcregress.pl taptest ./src/test/authentication/
-  test_recovery_script: |
-    %T_C% perl src/tools/msvc/vcregress.pl recoverycheck
-  test_bin_script: |
-    %T_C% perl src/tools/msvc/vcregress.pl bincheck
+  test_tap_script: |
+    %T_C% perl src/tools/msvc/vcregress.pl alltaptests
   test_pg_upgrade_script: |
     %T_C% perl src/tools/msvc/vcregress.pl upgradecheck
   test_ecpg_script: |
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index 74aa572d2f..e27c1d15e5 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -49,7 +49,7 @@ if (-e "src/tools/msvc/buildenv.pl")
 
 my $what = shift || "";
 if ($what =~
-	/^(check|installcheck|plcheck|contribcheck|modulescheck|ecpgcheck|isolationcheck|upgradecheck|bincheck|recoverycheck|taptest)$/i
+	/^(check|installcheck|plcheck|contribcheck|modulescheck|ecpgcheck|isolationcheck|upgradecheck|bincheck|recoverycheck|taptest|alltaptests)$/i
   )
 {
 	$what = uc $what;
@@ -107,6 +107,7 @@ my %command = (
 	BINCHECK       => \&bincheck,
 	RECOVERYCHECK  => \&recoverycheck,
 	UPGRADECHECK   => \&upgradecheck,
+	ALLTAPTESTS    => \&alltaptests,
 	TAPTEST        => \&taptest,);
 
 my $proc = $command{$what};
@@ -261,6 +262,9 @@ sub tap_check
 	$ENV{PG_REGRESS}    = "$topdir/$Config/pg_regress/pg_regress";
 	$ENV{REGRESS_SHLIB} = "$topdir/src/test/regress/regress.dll";
 
+	print "============================================================\n";
+	print "Checking @args\n";
+
 	rmtree('tmp_check');
 	system(@args);
 	my $status = $? >> 8;
@@ -273,8 +277,7 @@ sub bincheck
 
 	my $mstat = 0;
 
-	# Find out all the existing TAP tests by looking for t/ directories
-	# in the tree.
+	# Find the TAP tests by looking for t/ directories
 	my @bin_dirs = glob("$topdir/src/bin/*");
 
 	# Process each test
@@ -288,6 +291,36 @@ sub bincheck
 	return;
 }
 
+sub alltaptests
+{
+	InstallTemp();
+
+	my $mstat = 0;
+
+	# Find out all the existing TAP tests by looking for t/ directories
+	# in the tree.
+	my @tap_dirs = ();
+	my @top_dir = ($topdir);
+	File::Find::find(
+		{   wanted => sub {
+			/^t\z/s
+			  && $File::Find::name !~ /\/(kerberos|ldap|ssl|ssl_passphrase_callback)\// # opt-in
+			  && push(@tap_dirs, $File::Find::name);
+			}
+		},
+		@top_dir);
+
+	# Process each test
+	foreach my $test_path (@tap_dirs)
+	{
+		my $dir = dirname($test_path);
+		my $status = tap_check($dir);
+		$mstat ||= $status;
+	}
+	exit $mstat if $mstat;
+	return;
+}
+
 sub taptest
 {
 	my $dir = shift;
@@ -777,6 +810,7 @@ sub usage
 	print STDERR
 	  "Usage: vcregress.pl <mode> [<arg>]\n\n",
 	  "Options for <mode>:\n",
+	  "  alltaptests    run all tap tests (except kerberos, ldap, ssl, ssl_passphrase_callback)\n",
 	  "  bincheck       run tests of utilities in src/bin/\n",
 	  "  check          deploy instance and run regression tests on it\n",
 	  "  contribcheck   run tests of modules in contrib/\n",
@@ -792,6 +826,7 @@ sub usage
 	  "  serial         serial mode\n",
 	  "  parallel       parallel mode\n",
 	  "\nOption for <arg>: for taptest\n",
-	  "  TEST_DIR       (required) directory where tests reside\n";
+	  "  TEST_DIR       (required) directory where tests reside\n",
+	  "  PROVE_FLAGS    flags to pass to prove\n";
 	exit(1);
 }
-- 
2.17.1


--ibvzjYYg+QDzMCy1
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="0005-cirrus-include-hints-how-to-install-OS-packages.patch"



^ permalink  raw  reply  [nested|flat] 2+ messages in thread

* Re: post-freeze damage control
@ 2024-04-12 03:11 David Steele <[email protected]>
  0 siblings, 0 replies; 2+ messages in thread

From: David Steele @ 2024-04-12 03:11 UTC (permalink / raw)
  To: Robert Haas <[email protected]>; +Cc: Tomas Vondra <[email protected]>; Tom Kincaid <[email protected]>; Michael Paquier <[email protected]>; Andrey M. Borodin <[email protected]>; Alvaro Herrera <[email protected]>; Stefan Fercot <[email protected]>; pgsql-hackers



On 4/12/24 12:15, Robert Haas wrote:
> On Thu, Apr 11, 2024 at 5:48 PM David Steele <[email protected]> wrote:
>> But they'll try because it is a new pg_basebackup feature and they'll
>> assume it is there to be used. Maybe it would be a good idea to make it
>> clear in the documentation that significant tooling will be required to
>> make it work.
> 
> I don't agree with that idea. LOTS of what we ship takes a significant
> amount of effort to make it work. You may well need a connection
> pooler. You may well need a failover manager which may or may not be
> separate from your connection pooler. You need a backup tool. You need
> a replication management tool which may or may not be separate from
> your backup tool and may or may not be separate from your failover
> tool. You probably need various out-of-core connections for the
> programming languages you need. You may need a management tool, and
> you probably need a monitoring tool. Some of the tools you might
> choose to do all that stuff themselves have a whole bunch of complex
> dependencies. It's a mess.

The difference here is you *can* use Postgres without a connection 
pooler (I have many times) or failover (if downtime is acceptable) but 
most people would agree that you really *need* backup.

The backup tool should be clear and easy to use or misery will 
inevitably result. pg_basebackup is difficult enough to use and automate 
because it has no notion of a repository, no expiration, and no WAL 
handling just to name a few things. Now there is an even more advanced 
feature that is even harder to use. So, no, I really don't think this 
feature is practically usable by the vast majority of end users.

> Now, if someone were to say that we ought to talk about these issues
> in our documentation and maybe give people some ideas about how to get
> started, I would likely be in favor of that, modulo the small
> political problem that various people would want their solution to be
> the canonical one to which everyone gets referred. But I think it's
> wrong to pretend like this feature is somehow special, that it's
> somehow more raw or unfinished than tons of other things. I actually
> think it's significantly *better* than a lot of other things. If we
> add a disclaimer to the documentation saying "hey, this new
> incremental backup feature is half-finished garbage!", and meanwhile
> the documentation still says "hey, you can use cp as your
> archive_command," then we have completely lost our minds.

Fair point on cp, but that just points to an overall lack in our 
documentation and built-in backup/recovery tools in general.

> I also think that you're being more negative about this than the facts
> justify. As I said to several colleagues today, I *fully* acknowledge
> that you have a lot more practical experience in this area than I do,
> and a bunch of good ideas. I was really pleased to see you talking
> about how it would be good if these tools worked on tar files - and I
> completely agree, and I hope that will happen, and I hope to help in
> making that happen. I think there are a bunch of other problems too,
> only some of which I can guess at. However, I think saying that this
> feature is not realistically intended to be used by end-users or that
> they will not be able to do so is over the top, and is actually kind
> of insulting. 

It is not meant to be insulting, but I still believe it to be true. 
After years of working with users on backup problems I think I have a 
pretty good bead on what the vast majority of admins are capable of 
and/or willing to do. Making this feature work is pretty high above that 
bar.

If the primary motivation is to provide a feature that can be integrated 
with third party tools, as Tomas suggests, then I guess usability is 
somewhat moot. But you are insisting that is not the case and I just 
don't see it that way.

> There has been more enthusiasm for this feature on this
> mailing list and elsewhere than I've gotten for anything I've
> developed in years. And I don't think that's because all of the people
> who have expressed enthusiasm are silly geese who don't understand how
> terrible it is.

No doubt there is enthusiasm. It's a great feature to have. In 
particular I think the WAL summarizer is cool. But I do think the 
shortcomings are significant and that will become very apparent when 
people start to implement. The last minute effort to add COW support is 
an indication of problems that people will see in the field.

Further, I do think some less that ideal design decisions were made. In 
particular, I think sidelining manifests, i.e. making them optional, is 
not a good choice. This has led directly to the issue we see in [1]. If 
we require a manifest to make an incremental backup, why make it 
optional for combine?

This same design decision has led us to have "marker files" for 
zero-length files and unchanged files, which just seems extremely 
wasteful when these could be noted in the manifest. There are good 
reasons for writing everything out in a full backup, but for an 
incremental that can only be reconstructed using our tool the manifest 
should be sufficient.

Maybe all of this can be improved in a future release, along with tar 
reading, but none of those potential future improvements help me to 
believe that this is a user-friendly feature in this release.

Regards,
-David

---

[1] 
https://www.postgresql.org/message-id/flat/9badd24d-5bd9-4c35-ba85-4c38a2feb73e%40pgmasters.net






^ permalink  raw  reply  [nested|flat] 2+ messages in thread


end of thread, other threads:[~2024-04-12 03:11 UTC | newest]

Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2022-02-25 23:00 [PATCH 4/7] vcregress: add alltaptests Justin Pryzby <[email protected]>
2024-04-12 03:11 Re: post-freeze damage control David Steele <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox