unofficial mirror of meta@public-inbox.org
 help / color / mirror / Atom feed
* [PATCH 0/6] test updates + coderepo removal workaround
@ 2024-09-30 21:30 Eric Wong
  2024-09-30 21:30 ` [PATCH 1/6] t/www_static: modernize test Eric Wong
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Eric Wong @ 2024-09-30 21:30 UTC (permalink / raw)
  To: meta

For instances using -httpd||-netd, coderepos disappearing should
be better handled now.

We should probably handle disappearing inboxes at some point, too.

I also have yet to test new coderepos appearing, but I seem
to recall that works for projects.list users...

Inboxes appearing while -httpd||-netd is running is another
question that's unresolved, since we need to load the entire
config (and associated system-wide settings to deal with it).

Eric Wong (6):
  t/www_static: modernize test
  t/www_static: test with our -httpd server, too
  t/www_static: ensure If-Modified-Since handling works
  www: improve handling of missing coderepos
  xt/solver: use `psgi' shortcut for require_mods()
  t/{config,solver_git}: simplify error handling

 lib/PublicInbox/Config.pm     |   7 ++-
 lib/PublicInbox/Git.pm        |   4 +-
 lib/PublicInbox/TestCommon.pm |   6 +-
 t/config.t                    |  18 +++---
 t/solver_git.t                |  66 +++++++++++++++-------
 t/www_static.t                | 102 +++++++++++++++++++++++++---------
 xt/solver.t                   |   4 +-
 7 files changed, 145 insertions(+), 62 deletions(-)

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

* [PATCH 1/6] t/www_static: modernize test
  2024-09-30 21:30 [PATCH 0/6] test updates + coderepo removal workaround Eric Wong
@ 2024-09-30 21:30 ` Eric Wong
  2024-09-30 21:30 ` [PATCH 2/6] t/www_static: test with our -httpd server, too Eric Wong
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Eric Wong @ 2024-09-30 21:30 UTC (permalink / raw)
  To: meta

autodie is standard in Perl v5.10+ and we now have
PublicInbox::IO::write_file to denoise test setup code.
Then we'll favor the non-wantarray calls to tmpdir() and
rely on an overloaded stringification rather than keeping
the $for_destroy object around.
---
 t/www_static.t | 36 ++++++++++++++++--------------------
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/t/www_static.t b/t/www_static.t
index 83a96a5e..48d120d6 100644
--- a/t/www_static.t
+++ b/t/www_static.t
@@ -1,17 +1,17 @@
-# Copyright (C) 2019-2021 all contributors <meta@public-inbox.org>
+#!perl -w
+# Copyright (C) all contributors <meta@public-inbox.org>
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
-use strict;
-use warnings;
-use Test::More;
+use v5.12;
+use autodie;
 use PublicInbox::TestCommon;
-my ($tmpdir, $for_destroy) = tmpdir();
-my @mods = qw(HTTP::Request::Common Plack::Test URI::Escape);
+use PublicInbox::IO qw(write_file);
+my $tmpdir = tmpdir();
 require_mods qw(psgi);
 require IO::Uncompress::Gunzip;
 use_ok 'PublicInbox::WwwStatic';
 
 my $app = sub {
-	my $ws = PublicInbox::WwwStatic->new(docroot => $tmpdir, @_);
+	my $ws = PublicInbox::WwwStatic->new(docroot => "$tmpdir", @_);
 	sub { $ws->call(shift) };
 };
 
@@ -19,9 +19,7 @@ test_psgi($app->(), sub {
 	my $cb = shift;
 	my $res = $cb->(GET('/'));
 	is($res->code, 404, '404 on "/" by default');
-	open my $fh, '>', "$tmpdir/index.html" or die;
-	print $fh 'hi' or die;
-	close $fh or die;
+	write_file '>', "$tmpdir/index.html", 'hi';
 	$res = $cb->(GET('/'));
 	is($res->code, 200, '200 with index.html');
 	is($res->content, 'hi', 'default index.html returned');
@@ -41,8 +39,8 @@ test_psgi($app->(autoindex => 1, index => []), sub {
 	my $ls = $res->content;
 	like($ls, qr/index\.html/, 'got listing with index.html');
 	ok(index($ls, $updir) < 0, 'no updir at /');
-	mkdir("$tmpdir/dir") or die;
-	rename("$tmpdir/index.html", "$tmpdir/dir/index.html") or die;
+	mkdir "$tmpdir/dir";
+	rename "$tmpdir/index.html", "$tmpdir/dir/index.html";
 
 	$res = $cb->(GET('/dir/'));
 	is($res->code, 200, '200 with autoindex for dir/');
@@ -58,8 +56,8 @@ test_psgi($app->(autoindex => 1, index => []), sub {
 	like($res->header('Location'), qr!://[^/]+/dir/\z!,
 		'redirected w/ slash');
 
-	rename("$tmpdir/dir/index.html", "$tmpdir/dir/foo") or die;
-	link("$tmpdir/dir/foo", "$tmpdir/dir/foo.gz") or die;
+	rename "$tmpdir/dir/index.html", "$tmpdir/dir/foo";
+	link "$tmpdir/dir/foo", "$tmpdir/dir/foo.gz";
 	$res = $cb->(GET('/dir/'));
 	unlike($res->content, qr/>foo\.gz</,
 		'.gz file hidden if mtime matches uncompressed');
@@ -68,15 +66,13 @@ test_psgi($app->(autoindex => 1, index => []), sub {
 	$res = $cb->(GET('/dir/foo/bar'));
 	is($res->code, 404, 'using file as dir fails');
 
-	unlink("$tmpdir/dir/foo") or die;
+	unlink "$tmpdir/dir/foo";
 	$res = $cb->(GET('/dir/'));
 	like($res->content, qr/>foo\.gz</,
 		'.gz shown when no uncompressed version exists');
 
-	open my $fh, '>', "$tmpdir/dir/foo" or die;
-	print $fh "uncompressed\n" or die;
-	close $fh or die;
-	utime(0, 0, "$tmpdir/dir/foo") or die;
+	write_file '>', "$tmpdir/dir/foo", "uncompressed\n";
+	utime 0, 0, "$tmpdir/dir/foo";
 	$res = $cb->(GET('/dir/'));
 	my $html = $res->content;
 	like($html, qr/>foo</, 'uncompressed foo shown');
@@ -86,7 +82,7 @@ test_psgi($app->(autoindex => 1, index => []), sub {
 	is($res->content, "uncompressed\n",
 		'got uncompressed on mtime mismatch');
 
-	utime(0, 0, "$tmpdir/dir/foo.gz") or die;
+	utime 0, 0, "$tmpdir/dir/foo.gz";
 	my $get = GET('/dir/foo');
 	$get->header('Accept-Encoding' => 'gzip');
 	$res = $cb->($get);

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

* [PATCH 2/6] t/www_static: test with our -httpd server, too
  2024-09-30 21:30 [PATCH 0/6] test updates + coderepo removal workaround Eric Wong
  2024-09-30 21:30 ` [PATCH 1/6] t/www_static: modernize test Eric Wong
@ 2024-09-30 21:30 ` Eric Wong
  2024-09-30 21:30 ` [PATCH 3/6] t/www_static: ensure If-Modified-Since handling works Eric Wong
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Eric Wong @ 2024-09-30 21:30 UTC (permalink / raw)
  To: meta

While our current HTTP implementation doesn't make special
allowances for static files, it only hurts a little to fork
off -httpd instances and ensure any future changes work as
expected.
---
 lib/PublicInbox/TestCommon.pm |  6 +++++-
 t/www_static.t                | 32 +++++++++++++++++++++++++-------
 2 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/lib/PublicInbox/TestCommon.pm b/lib/PublicInbox/TestCommon.pm
index 913aa7e9..6c3677d2 100644
--- a/lib/PublicInbox/TestCommon.pm
+++ b/lib/PublicInbox/TestCommon.pm
@@ -949,17 +949,21 @@ sub create_inbox ($;@) {
 sub test_httpd ($$;$$) {
 	my ($env, $client, $skip, $cb) = @_;
 	my ($tmpdir, $for_destroy);
+	my $psgi = delete $env->{psgi_file};
+	if (!defined $psgi) {
+		for (qw(PI_CONFIG)) { $env->{$_} or BAIL_OUT "$_ unset" }
+	}
 	$env->{TMPDIR} //= do {
 		($tmpdir, $for_destroy) = tmpdir();
 		$tmpdir;
 	};
-	for (qw(PI_CONFIG)) { $env->{$_} or BAIL_OUT "$_ unset" }
 	SKIP: {
 		require_mods qw(Plack::Test::ExternalServer LWP::UserAgent
 				-httpd), $skip // 1;
 		my $sock = tcp_server() or die;
 		my ($out, $err) = map { "$env->{TMPDIR}/std$_.log" } qw(out err);
 		my $cmd = [ qw(-httpd -W0), "--stdout=$out", "--stderr=$err" ];
+		push @$cmd, $psgi if defined $psgi;
 		my $td = start_script($cmd, $env, { 3 => $sock });
 		my ($h, $p) = tcp_host_port($sock);
 		local $ENV{PLACK_TEST_EXTERNALSERVER_URI} = "http://$h:$p";
diff --git a/t/www_static.t b/t/www_static.t
index 48d120d6..b1181cab 100644
--- a/t/www_static.t
+++ b/t/www_static.t
@@ -8,15 +8,23 @@ use PublicInbox::IO qw(write_file);
 my $tmpdir = tmpdir();
 require_mods qw(psgi);
 require IO::Uncompress::Gunzip;
+use File::Path qw(remove_tree);
 use_ok 'PublicInbox::WwwStatic';
 
-my $app = sub {
-	my $ws = PublicInbox::WwwStatic->new(docroot => "$tmpdir", @_);
-	sub { $ws->call(shift) };
+my $psgi_env = sub { # @_ is passed to WwwStatic->new
+	my $ret = "$tmpdir/www_static.psgi";
+	write_file '>', $ret, <<EOM;
+use v5.12;
+use Plack::Builder;
+my \$ws = PublicInbox::WwwStatic->new(docroot => "$tmpdir" @_);
+builder { sub { \$ws->call(shift) } }
+EOM
+	{ psgi_file => $ret, TMPDIR => "$tmpdir" };
 };
 
-test_psgi($app->(), sub {
+my $client = sub {
 	my $cb = shift;
+	unlink "$tmpdir/index.html" if -f "$tmpdir/index.html";
 	my $res = $cb->(GET('/'));
 	is($res->code, 404, '404 on "/" by default');
 	write_file '>', "$tmpdir/index.html", 'hi';
@@ -29,10 +37,15 @@ test_psgi($app->(), sub {
 	is($res->header('Content-Length'), '2', 'content-length set');
 	like($res->header('Content-Type'), qr!^text/html\b!,
 		'content-type is html');
-});
+};
+
+my $env = $psgi_env->();
+test_psgi(Plack::Util::load_psgi($env->{psgi_file}), $client);
+test_httpd $env, $client;
 
-test_psgi($app->(autoindex => 1, index => []), sub {
+$client = sub {
 	my $cb = shift;
+	write_file '>', "$tmpdir/index.html", 'hi';
 	my $res = $cb->(GET('/'));
 	my $updir = 'href="../">../</a>';
 	is($res->code, 200, '200 with autoindex default');
@@ -96,6 +109,11 @@ test_psgi($app->(autoindex => 1, index => []), sub {
 	IO::Uncompress::Gunzip::gunzip(\$in => \$out);
 	like($out, qr/\A<html>/, 'got HTML start after gunzip');
 	like($out, qr{</html>$}, 'got HTML end after gunzip');
-});
+	remove_tree "$tmpdir/dir";
+};
+
+$env = $psgi_env->(', autoindex => 1, index => []');
+test_psgi(Plack::Util::load_psgi($env->{psgi_file}), $client);
+test_httpd $env, $client;
 
 done_testing();

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

* [PATCH 3/6] t/www_static: ensure If-Modified-Since handling works
  2024-09-30 21:30 [PATCH 0/6] test updates + coderepo removal workaround Eric Wong
  2024-09-30 21:30 ` [PATCH 1/6] t/www_static: modernize test Eric Wong
  2024-09-30 21:30 ` [PATCH 2/6] t/www_static: test with our -httpd server, too Eric Wong
@ 2024-09-30 21:30 ` Eric Wong
  2024-09-30 21:30 ` [PATCH 4/6] www: improve handling of missing coderepos Eric Wong
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Eric Wong @ 2024-09-30 21:30 UTC (permalink / raw)
  To: meta

We've had If-Modified-Since to reduce client traffic for a
while, so ensure it's tested properly and continues to work
in the future.
---
 t/www_static.t | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/t/www_static.t b/t/www_static.t
index b1181cab..8fb86a82 100644
--- a/t/www_static.t
+++ b/t/www_static.t
@@ -109,6 +109,42 @@ $client = sub {
 	IO::Uncompress::Gunzip::gunzip(\$in => \$out);
 	like($out, qr/\A<html>/, 'got HTML start after gunzip');
 	like($out, qr{</html>$}, 'got HTML end after gunzip');
+	unlink "$tmpdir/dir/foo.gz";
+	$get = GET('/dir/foo');
+
+	require HTTP::Date;
+	HTTP::Date->import('time2str');
+	$get->header('If-Modified-Since' => time2str(0));
+	$res = $cb->($get);
+	is $res->code, 304, '304 on If-Modified-Since hit';
+	$get->header('If-Modified-Since' => time2str(1));
+	$res = $cb->($get);
+	is $res->code, 200, '200 on If-Modified-Since miss';
+SKIP: {
+	# validating with curl ensures we didn't carry the same
+	# misunderstandings across both the code being tested
+	# and the test itself.
+	$ENV{TEST_EXPENSIVE} or
+		skip 'TEST_EXPENSIVE unset for validation w/curl', 1;
+	my $uri = $ENV{PLACK_TEST_EXTERNALSERVER_URI} or
+		skip 'curl test skipped w/o external server', 1;
+	my $dst = "$tmpdir/foo.dst";
+	my $dst2 = "$dst.2";
+	$uri .= '/dir/foo';
+	state $curl = require_cmd 'curl', 1;
+	xsys_e $curl, '-gsSfR', '-o', $dst, $uri;
+	xsys_e [ $curl, '-vgsSfR', '-o', $dst2, $uri, '-z', $dst ],
+		undef, { 2 => \(my $curl_err) };
+	like $curl_err, qr! HTTP/1\.[012] 304 !sm,
+		'got 304 response w/ If-Modified-Since';
+	is -s $dst2, undef, 'nothing downloaded on 304';
+	utime 1, 1, "$tmpdir/dir/foo";
+	xsys_e [ $curl, '-vgsSfR', '-o', $dst2, $uri, '-z', $dst ],
+		undef, { 2 => \$curl_err };
+	is -s $dst2, -s $dst, 'got 200 on If-Modified-Since mismatch';
+	like $curl_err, qr! HTTP/1\.[012] 200 !sm,
+		'got 200 response w/ If-Modified-Since';
+} # SKIP
 	remove_tree "$tmpdir/dir";
 };
 

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

* [PATCH 4/6] www: improve handling of missing coderepos
  2024-09-30 21:30 [PATCH 0/6] test updates + coderepo removal workaround Eric Wong
                   ` (2 preceding siblings ...)
  2024-09-30 21:30 ` [PATCH 3/6] t/www_static: ensure If-Modified-Since handling works Eric Wong
@ 2024-09-30 21:30 ` Eric Wong
  2024-09-30 21:30 ` [PATCH 5/6] xt/solver: use `psgi' shortcut for require_mods() Eric Wong
  2024-09-30 21:30 ` [PATCH 6/6] t/{config,solver_git}: simplify error handling Eric Wong
  5 siblings, 0 replies; 7+ messages in thread
From: Eric Wong @ 2024-09-30 21:30 UTC (permalink / raw)
  To: meta

Git coderepos may appear and disappear during the lifetime of
an -httpd or -netd instance.  This happens quite frequently on
on my git.kernel.org mirror and is and clutters up stderr, so
we'll validate the existance of a git directory before trying
to serve anything.

Whether or not this should be the case for inboxes is yet-to-be
decided, especially since there's inbox-specific information
inside in PI_CONFIG (e.g. address, newsgroup) and we only use a
project listing for coderepos.
---
 lib/PublicInbox/Config.pm |  7 +++++--
 lib/PublicInbox/Git.pm    |  4 +++-
 t/config.t                |  5 +++--
 t/solver_git.t            | 34 ++++++++++++++++++++++++++++++++--
 4 files changed, 43 insertions(+), 7 deletions(-)

diff --git a/lib/PublicInbox/Config.pm b/lib/PublicInbox/Config.pm
index d76deca4..442759de 100644
--- a/lib/PublicInbox/Config.pm
+++ b/lib/PublicInbox/Config.pm
@@ -375,7 +375,9 @@ sub valid_dir ($$) {
 sub fill_coderepo {
 	my ($self, $nick) = @_;
 	my $pfx = "coderepo.$nick";
-	my $git = PublicInbox::Git->new(valid_dir($self, "$pfx.dir") // return);
+	my $git_dir = valid_dir($self, "$pfx.dir") // return;
+	-e $git_dir // return;
+	my $git = PublicInbox::Git->new($git_dir);
 	if (defined(my $cgits = $self->{"$pfx.cgiturl"})) {
 		$git->{cgit_url} = $cgits = _array($cgits);
 		$self->{"$pfx.cgiturl"} = $cgits;
@@ -701,13 +703,14 @@ sub glob2re ($) {
 
 sub get_coderepo {
 	my ($self, $nick) = @_;
-	$self->{-coderepos}->{$nick} // do {
+	my $git = $self->{-coderepos}->{$nick} // do {
 		defined($self->{-cgit_scan_path}) ? do {
 			apply_cgit_scan_path($self);
 			my $cr = fill_coderepo($self, $nick);
 			$cr ? ($self->{-coderepos}->{$nick} = $cr) : undef;
 		} : undef;
 	};
+	$git && -e $git->{git_dir} ? $git : undef;
 }
 
 1;
diff --git a/lib/PublicInbox/Git.pm b/lib/PublicInbox/Git.pm
index c43f9863..b3f39adf 100644
--- a/lib/PublicInbox/Git.pm
+++ b/lib/PublicInbox/Git.pm
@@ -250,7 +250,9 @@ sub cat_async_step ($$) {
 	my $cmd = ref($req) ? $$req : $req;
 	# ->fail may be called via Gcf2Client.pm
 	my $info = $self->{-bc} && substr($cmd, 0, 5) eq 'info ';
-	if ($head =~ /^([0-9a-f]{40,}) (\S+) ([0-9]+)$/) {
+	if (!defined $head) {
+		$self->fail("E: $self->{git_dir} gone? (\$!=$!)");
+	} elsif ($head =~ /^([0-9a-f]{40,}) (\S+) ([0-9]+)$/) {
 		($oid, $type, $size) = ($1, $2, $3 + 0);
 		unless ($info) { # --batch-command
 			$bref = $self->{sock}->my_bufread($size + 1) or
diff --git a/t/config.t b/t/config.t
index c41a42d3..956ad35c 100644
--- a/t/config.t
+++ b/t/config.t
@@ -7,7 +7,7 @@ use_ok 'PublicInbox';
 ok(defined(eval('$PublicInbox::VERSION')), 'VERSION defined');
 use_ok 'PublicInbox::Config';
 my ($tmpdir, $for_destroy) = tmpdir();
-use autodie qw(open close);
+use autodie qw(close mkdir open);
 my $validate_git_behavior = $ENV{TEST_VALIDATE_GIT_BEHAVIOR};
 
 {
@@ -240,8 +240,9 @@ for my $s (@valid) {
 	inboxdir = /path/to/foo
 	coderepo = project
 [coderepo "project"]
-	dir = /path/to/project.git
+	dir = $tmpdir/project.git
 EOF
+	mkdir "$tmpdir/project.git"; # must exist for ->fill_coderepo
 	my $t1 = $cfg->lookup_name('test1');
 	my $t2 = $cfg->lookup_name('test2');
 	ok $cfg->repo_objs($t1)->[0], 'coderepo parsed';
diff --git a/t/solver_git.t b/t/solver_git.t
index 694e2671..6cc5eeba 100644
--- a/t/solver_git.t
+++ b/t/solver_git.t
@@ -6,8 +6,11 @@ use PublicInbox::TestCommon;
 use Cwd qw(abs_path);
 require_git v2.6;
 use PublicInbox::ContentHash qw(git_sha);
-use PublicInbox::Spawn qw(run_qx);
-require_mods(qw(DBD::SQLite Xapian));
+use PublicInbox::Spawn qw(run_qx which);
+use File::Path qw(remove_tree);
+use PublicInbox::IO qw(write_file);
+use autodie qw(symlink unlink);
+require_mods qw(DBD::SQLite Xapian);
 require PublicInbox::SolverGit;
 my $rdr = { 2 => \(my $null) };
 my $git_dir = xqx([qw(git rev-parse --git-common-dir)], undef, $rdr);
@@ -22,6 +25,8 @@ my $patch2 = eml_load 't/solve/0002-rename-with-modifications.patch';
 my $patch2_oid = git_sha(1, $patch2)->hexdigest;
 
 my ($tmpdir, $for_destroy) = tmpdir();
+my $gone_repo = "$tmpdir/to-be-deleted.git";
+
 my $ibx = create_inbox 'v2', version => 2,
 			indexlevel => 'medium', sub {
 	my ($im) = @_;
@@ -287,6 +292,8 @@ SKIP: {
 [coderepo "binfoo"]
 	dir = $binfoo
 	cgiturl = http://example.com/binfoo
+[coderepo "goner"]
+	dir = $gone_repo
 EOF
 	close $cfgfh or die;
 	my $exp_digest;
@@ -450,9 +457,32 @@ EOF
 			my $t = XML::TreePP->new->parse($res->content);
 			ok(scalar @{$t->{feed}->{entry}}, 'got tag entries');
 		}
+
+		# test disappearing repos
+		$res = $cb->(GET('/goner/'));
+		is $res->code, 200, 'coderepo summary for goner repo';
+		$res = $cb->(GET("/goner/$oid{'iso-8859-1'}/s/"));
+		is $res->code, 200, 'goner repo OID /s/ still available';
+		$stderr_empty->();
+		remove_tree $gone_repo;
+
+		$res = $cb->(GET('/goner/'));
+		is $res->code, 404, '/goner/ repo summary';
+		$stderr_empty->();
+
+		$res = $cb->(GET("/goner/$oid{'iso-8859-1'}/s/"));
+		is $res->code, 404, 'gone repo OID /s/';
+		$stderr_empty->();
 	};
+
+	state $cp = which('cp');
+	$cp or xbail "`cp' not available (WTF!?)";
+	xsys_e $cp, qw(-Rp), $binfoo, $gone_repo;
+
 	test_psgi(sub { $www->call(@_) }, $client);
 	my $env = { PI_CONFIG => $cfgpath, TMPDIR => $tmpdir };
+
+	xsys_e $cp, qw(-Rp), $binfoo, $gone_repo; # for test_httpd $client
 	test_httpd($env, $client, 7, sub {
 	SKIP: {
 		require_cmd('curl', 1) or skip 'no curl', 1;

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

* [PATCH 5/6] xt/solver: use `psgi' shortcut for require_mods()
  2024-09-30 21:30 [PATCH 0/6] test updates + coderepo removal workaround Eric Wong
                   ` (3 preceding siblings ...)
  2024-09-30 21:30 ` [PATCH 4/6] www: improve handling of missing coderepos Eric Wong
@ 2024-09-30 21:30 ` Eric Wong
  2024-09-30 21:30 ` [PATCH 6/6] t/{config,solver_git}: simplify error handling Eric Wong
  5 siblings, 0 replies; 7+ messages in thread
From: Eric Wong @ 2024-09-30 21:30 UTC (permalink / raw)
  To: meta

The `psgi' shortcut simplifies setup, reduces the likelyhood
of human error from omitted modules, and avoid needless `use_ok'
tests which make the output noisier than necessary.
---
 xt/solver.t | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/xt/solver.t b/xt/solver.t
index 372d003b..d074b1b5 100644
--- a/xt/solver.t
+++ b/xt/solver.t
@@ -4,9 +4,7 @@
 use v5.12;
 use PublicInbox::TestCommon;
 use PublicInbox::Config; # this relies on PI_CONFIG // ~/.public-inbox/config
-my @psgi = qw(HTTP::Request::Common Plack::Test URI::Escape Plack::Builder);
-require_mods(qw(DBD::SQLite Xapian), @psgi);
-use_ok($_) for @psgi;
+require_mods qw(DBD::SQLite Xapian psgi);
 use_ok 'PublicInbox::WWW';
 my $cfg = PublicInbox::Config->new;
 my $www = PublicInbox::WWW->new($cfg);

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

* [PATCH 6/6] t/{config,solver_git}: simplify error handling
  2024-09-30 21:30 [PATCH 0/6] test updates + coderepo removal workaround Eric Wong
                   ` (4 preceding siblings ...)
  2024-09-30 21:30 ` [PATCH 5/6] xt/solver: use `psgi' shortcut for require_mods() Eric Wong
@ 2024-09-30 21:30 ` Eric Wong
  5 siblings, 0 replies; 7+ messages in thread
From: Eric Wong @ 2024-09-30 21:30 UTC (permalink / raw)
  To: meta

autodie and the newish PublicInbox::IO::write_file make error
handling more consistent and less noisy to people reading the
code.  We'll also avoid testing `git config' set behavior
of git(1) and instead bail out via `xsys_e()' if it fails
unexpectedly due to hardware problems or bugs in git.
---
 t/config.t     | 13 ++++++-------
 t/solver_git.t | 34 +++++++++++++++-------------------
 2 files changed, 21 insertions(+), 26 deletions(-)

diff --git a/t/config.t b/t/config.t
index 956ad35c..85005df9 100644
--- a/t/config.t
+++ b/t/config.t
@@ -3,6 +3,7 @@
 use v5.12;
 use PublicInbox::TestCommon;
 use PublicInbox::Import;
+use PublicInbox::IO qw(write_file);
 use_ok 'PublicInbox';
 ok(defined(eval('$PublicInbox::VERSION')), 'VERSION defined');
 use_ok 'PublicInbox::Config';
@@ -12,13 +13,11 @@ my $validate_git_behavior = $ENV{TEST_VALIDATE_GIT_BEHAVIOR};
 
 {
 	my $f = "$tmpdir/bool_config";
-	open my $fh, '>', $f;
-	print $fh <<EOM;
+	write_file '>', $f, <<EOM;
 [imap]
 	debug
 	port = 2
 EOM
-	close $fh;
 	my $cfg = PublicInbox::Config->git_config_dump($f);
 	$validate_git_behavior and
 		is(xqx([qw(git config -f), $f, qw(--bool imap.debug)]),
@@ -32,7 +31,7 @@ EOM
 	my $inboxdir = "$tmpdir/new\nline";
 	my @cmd = ('git', "--git-dir=$tmpdir",
 		qw(config publicinbox.foo.inboxdir), $inboxdir);
-	is(xsys(@cmd), 0, "set config");
+	xsys_e(@cmd);
 
 	my $tmp = PublicInbox::Config->new("$tmpdir/config");
 
@@ -212,17 +211,17 @@ for my $s (@valid) {
 
 {
 	my $f = "$tmpdir/ordered";
-	open my $fh, '>', $f or die "open: $!";
+	open my $fh, '>', $f;
 	my @expect;
 	foreach my $i (0..3) {
 		push @expect, "$i";
-		print $fh <<"" or die "print: $!";
+		print $fh <<"";
 [publicinbox "$i"]
 	inboxdir = /path/to/$i.git
 	address = $i\@example.com
 
 	}
-	close $fh or die "close: $!";
+	close $fh;
 	my $cfg = PublicInbox::Config->new($f);
 	my @result;
 	$cfg->each_inbox(sub { push @result, $_[0]->{name} });
diff --git a/t/solver_git.t b/t/solver_git.t
index 6cc5eeba..cebcf47f 100644
--- a/t/solver_git.t
+++ b/t/solver_git.t
@@ -9,7 +9,7 @@ use PublicInbox::ContentHash qw(git_sha);
 use PublicInbox::Spawn qw(run_qx which);
 use File::Path qw(remove_tree);
 use PublicInbox::IO qw(write_file);
-use autodie qw(symlink unlink);
+use autodie qw(close mkdir open rename symlink unlink);
 require_mods qw(DBD::SQLite Xapian);
 require PublicInbox::SolverGit;
 my $rdr = { 2 => \(my $null) };
@@ -35,8 +35,7 @@ my $ibx = create_inbox 'v2', version => 2,
 };
 my $md = "$tmpdir/md";
 File::Path::make_path(map { $md.$_ } (qw(/cur /new /tmp)));
-symlink(abs_path('t/solve/0001-simple-mod.patch'), "$md/cur/foo:2,") or
-	xbail "symlink: $!";
+symlink abs_path('t/solve/0001-simple-mod.patch'), "$md/cur/foo:2,";
 
 my $v1_0_0_rev = '8a918a8523bc9904123460f85999d75f6d604916';
 my $v1_0_0_tag = 'cb7c42b1e15577ed2215356a2bf925aef59cdd8d';
@@ -45,7 +44,7 @@ my $expect = '69df7d565d49fbaaeb0a067910f03dc22cd52bd0';
 my $non_existent = 'ee5e32211bf62ab6531bdf39b84b6920d0b6775a';
 my $stderr_empty = sub {
 	my ($msg) = @_;
-	open my $efh, '<', "$tmpdir/stderr.log" or xbail $!;
+	open my $efh, '<', "$tmpdir/stderr.log";
 	my @l = <$efh>;
 	@l = grep(!/reverse ?proxy/i, @l);
 	is_xdeeply(\@l, [], $msg // 'stderr.log is empty');
@@ -158,7 +157,7 @@ my $git = PublicInbox::Git->new($git_dir);
 $ibx->{-repo_objs} = [ $git ];
 my $res;
 my $solver = PublicInbox::SolverGit->new($ibx, sub { $res = $_[0] });
-open my $log, '+>>', "$tmpdir/solve.log" or die "open: $!";
+open my $log, '+>>', "$tmpdir/solve.log";
 my $psgi_env = { 'psgi.errors' => \*STDERR, 'psgi.url_scheme' => 'http',
 		'HTTP_HOST' => 'example.com' };
 $solver->solve($psgi_env, $log, '69df7d5', {});
@@ -230,7 +229,7 @@ SKIP: {
 	my $lk = PublicInbox::Lock->new($l);
 	my $acq = $lk->lock_for_scope;
 	my $stamp = "$binfoo/stamp-";
-	if (open my $fh, '<', $stamp) {
+	if (CORE::open my $fh, '<', $stamp) {
 		%oid = map { chomp; split(/=/, $_) } (<$fh>);
 	} else {
 		PublicInbox::Import::init_bare($binfoo);
@@ -243,7 +242,7 @@ SKIP: {
 			$oid{$label} = $x;
 		}
 
-		open my $null, '<', '/dev/null' or xbail "open /dev/null: $!";
+		open my $null, '<', '/dev/null';
 		my $t = xqx([qw(git mktree)], $env, { 0 => $null });
 		xbail "mktree: $?" if $?;
 		chomp($t);
@@ -264,20 +263,19 @@ SKIP: {
 		chomp($c);
 		$oid{'8859-parent'} = $c;
 
-		open my $fh, '>', "$stamp.$$" or BAIL_OUT;
+		open my $fh, '>', "$stamp.$$";
 		while (my ($k, $v) = each %oid) {
-			print $fh "$k=$v\n" or xbail "print: $!";
+			print $fh "$k=$v\n";
 		}
-		close $fh or BAIL_OUT;
-		rename("$stamp.$$", $stamp) or BAIL_OUT;
+		close $fh;
+		rename "$stamp.$$", $stamp;
 	}
 	undef $acq;
 	# ensure the PSGI frontend (ViewVCS) works:
 	my $name = $ibx->{name};
 	my $cfgpfx = "publicinbox.$name";
 	my $cfgpath = "$tmpdir/httpd-config";
-	open my $cfgfh, '>', $cfgpath or die;
-	print $cfgfh <<EOF or die;
+	write_file '>', $cfgpath, <<EOF;
 [coderepo]
 	snapshots = tar.gz
 [publicinbox "$name"]
@@ -295,7 +293,6 @@ SKIP: {
 [coderepo "goner"]
 	dir = $gone_repo
 EOF
-	close $cfgfh or die;
 	my $exp_digest;
 	{
 		my $exp = xqx([qw(git archive --format=tar.gz
@@ -354,13 +351,12 @@ EOF
 		if (defined $ENV{PLACK_TEST_EXTERNALSERVER_URI}) {
 			$stderr_empty->('nothing in stderr.log, yet');
 		} else {
-			open $olderr, '>&', \*STDERR or xbail "open: $!";
-			open STDERR, '+>>', "$tmpdir/stderr.log" or
-				xbail "open: $!";
+			open $olderr, '>&', \*STDERR;
+			open STDERR, '+>>', "$tmpdir/stderr.log";
 		}
 		$res = $cb->(GET('/binfoo/'));
 		defined($ENV{PLACK_TEST_EXTERNALSERVER_URI}) or
-			open STDERR, '>&', $olderr or xbail "open: $!";
+			open STDERR, '>&', $olderr;
 		is($res->code, 200, 'coderepo summary (binfoo)');
 		$stderr_empty->();
 
@@ -486,7 +482,7 @@ EOF
 	test_httpd($env, $client, 7, sub {
 	SKIP: {
 		require_cmd('curl', 1) or skip 'no curl', 1;
-		mkdir "$tmpdir/ext" // xbail "mkdir $!";
+		mkdir "$tmpdir/ext";
 		my $rurl = "$ENV{PLACK_TEST_EXTERNALSERVER_URI}/$name";
 		test_lei({tmpdir => "$tmpdir/ext"}, sub {
 			lei_ok(qw(blob --no-mail 69df7d5 -I), $rurl);

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

end of thread, other threads:[~2024-09-30 21:30 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-30 21:30 [PATCH 0/6] test updates + coderepo removal workaround Eric Wong
2024-09-30 21:30 ` [PATCH 1/6] t/www_static: modernize test Eric Wong
2024-09-30 21:30 ` [PATCH 2/6] t/www_static: test with our -httpd server, too Eric Wong
2024-09-30 21:30 ` [PATCH 3/6] t/www_static: ensure If-Modified-Since handling works Eric Wong
2024-09-30 21:30 ` [PATCH 4/6] www: improve handling of missing coderepos Eric Wong
2024-09-30 21:30 ` [PATCH 5/6] xt/solver: use `psgi' shortcut for require_mods() Eric Wong
2024-09-30 21:30 ` [PATCH 6/6] t/{config,solver_git}: simplify error handling Eric Wong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).