* [PATCH 0/4] fixes noticed while diagnosing t/lei-up.t
@ 2022-09-30 9:21 Eric Wong
2022-09-30 9:21 ` [PATCH 1/4] tests: favor 3 argument `open' with interopolation Eric Wong
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: Eric Wong @ 2022-09-30 9:21 UTC (permalink / raw)
To: meta
I'm still trying to figure out why OverIdx->adj_counter (via
next_tid) in the LeiSavedSearch dedupe check occasionally fails
under CentOS 7.x (but not other systems).
Meanwhile, some improvements noticed along the way.
The underlying problem remains...
Disabling WAL didn't help t/lei-up.t on CentOS 7.x, so the
obvious newish feature we use is unlikely the culprit...
Eric Wong (4):
tests: favor 3 argument `open' with interopolation
t/lei-up: improve diagnostics for this test
lei_to_mail: propagate errors to script/lei
t/altid_v2: improve test style
lib/PublicInbox/LeiToMail.pm | 34 ++++++++++++++++------------
t/altid_v2.t | 10 ++++-----
t/hl_mod.t | 4 ++--
t/lei-up.t | 43 ++++++++++++++++++++++--------------
t/lei_to_mail.t | 10 ++++-----
5 files changed, 59 insertions(+), 42 deletions(-)
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/4] tests: favor 3 argument `open' with interopolation
2022-09-30 9:21 [PATCH 0/4] fixes noticed while diagnosing t/lei-up.t Eric Wong
@ 2022-09-30 9:21 ` Eric Wong
2022-09-30 9:21 ` [PATCH 2/4] t/lei-up: improve diagnostics for this test Eric Wong
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Eric Wong @ 2022-09-30 9:21 UTC (permalink / raw)
To: meta
It makes code easier to review, and is more robust in case some
weirdos actually start their path names with '<' or '>' :P
---
t/hl_mod.t | 4 ++--
t/lei-up.t | 8 ++++----
t/lei_to_mail.t | 10 +++++-----
3 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/t/hl_mod.t b/t/hl_mod.t
index a88f6c03..6ddbb778 100644
--- a/t/hl_mod.t
+++ b/t/hl_mod.t
@@ -1,5 +1,5 @@
#!/usr/bin/perl -w
-# Copyright (C) 2019-2021 all contributors <meta@public-inbox.org>
+# Copyright (C) all contributors <meta@public-inbox.org>
# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
use strict; use v5.10.1; use PublicInbox::TestCommon; use IO::Handle; # ->autoflush
use Fcntl qw(:seek);
@@ -11,7 +11,7 @@ is($hls->_shebang2lang(\"#!/usr/bin/perl -w\n"), 'perl', 'perl shebang OK');
is($hls->{-ext2lang}->{'pm'}, 'perl', '.pm suffix OK');
is($hls->{-ext2lang}->{'pl'}, 'perl', '.pl suffix OK');
like($hls->_path2lang('Makefile'), qr/\Amake/, 'Makefile OK');
-my $str = do { local $/; open(my $fh, __FILE__); <$fh> };
+my $str = do { local $/; open(my $fh, '<', __FILE__); <$fh> };
my $orig = $str;
{
diff --git a/t/lei-up.t b/t/lei-up.t
index fc369156..022ebc05 100644
--- a/t/lei-up.t
+++ b/t/lei-up.t
@@ -15,23 +15,23 @@ test_lei(sub {
$s = eml_load('t/utf8.eml')->as_string;
lei_ok [qw(import -q -F eml -)], undef, { 0 => \$s, %$lei_opt };
lei_ok qw(up --all=local);
- open my $fh, "$ENV{HOME}/a.mbox.gz" or xbail "open: $!";
+ open my $fh, '<', "$ENV{HOME}/a.mbox.gz" or xbail "open: $!";
my $gz = do { local $/; <$fh> };
my $uc;
gunzip(\$gz => \$uc, MultiStream => 1) or xbail "gunzip $GunzipError";
- open $fh, "$ENV{HOME}/a" or xbail "open: $!";
+ open $fh, '<', "$ENV{HOME}/a" or xbail "open: $!";
my $exp = do { local $/; <$fh> };
is($uc, $exp, 'compressed and uncompressed match (a.gz)');
like($exp, qr/testmessage\@example.com/, '2nd message added');
- open $fh, "$ENV{HOME}/b.mbox.gz" or xbail "open: $!";
+ open $fh, '<', "$ENV{HOME}/b.mbox.gz" or xbail "open: $!";
$gz = do { local $/; <$fh> };
undef $uc;
gunzip(\$gz => \$uc, MultiStream => 1) or xbail "gunzip $GunzipError";
is($uc, $exp, 'compressed and uncompressed match (b.gz)');
- open $fh, "$ENV{HOME}/b" or xbail "open: $!";
+ open $fh, '<', "$ENV{HOME}/b" or xbail "open: $!";
$uc = do { local $/; <$fh> };
is($uc, $exp, 'uncompressed both match');
diff --git a/t/lei_to_mail.t b/t/lei_to_mail.t
index e8958c64..d692751c 100644
--- a/t/lei_to_mail.t
+++ b/t/lei_to_mail.t
@@ -1,5 +1,5 @@
#!perl -w
-# Copyright (C) 2020-2021 all contributors <meta@public-inbox.org>
+# Copyright (C) all contributors <meta@public-inbox.org>
# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
use strict;
use v5.10.1;
@@ -260,7 +260,7 @@ SKIP: { # FIFO support
my @f;
$mdr->maildir_each_file($md, sub { push @f, shift });
- open my $fh, $f[0] or BAIL_OUT $!;
+ open my $fh, '<', $f[0] or BAIL_OUT $!;
is(do { local $/; <$fh> }, $buf, 'wrote to Maildir');
$wcb = $wcb_get->('maildir', $md);
@@ -271,7 +271,7 @@ SKIP: { # FIFO support
$mdr->maildir_each_file($md, sub { push @x, shift });
is(scalar(@x), 1, 'wrote one new file');
ok(!-f $f[0], 'old file clobbered');
- open $fh, $x[0] or BAIL_OUT $!;
+ open $fh, '<', $x[0] or BAIL_OUT $!;
is(do { local $/; <$fh> }, $buf."\nx\n", 'wrote new file to Maildir');
local $lei->{opt}->{augment} = 1;
@@ -283,9 +283,9 @@ SKIP: { # FIFO support
is(scalar grep(/\A\Q$x[0]\E\z/, @f), 1, 'old file still there');
my @new = grep(!/\A\Q$x[0]\E\z/, @f);
is(scalar @new, 1, '1 new file written (b4dc0ffee skipped)');
- open $fh, $x[0] or BAIL_OUT $!;
+ open $fh, '<', $x[0] or BAIL_OUT $!;
is(do { local $/; <$fh> }, $buf."\nx\n", 'old file untouched');
- open $fh, $new[0] or BAIL_OUT $!;
+ open $fh, '<', $new[0] or BAIL_OUT $!;
is(do { local $/; <$fh> }, $buf."\ny\n", 'new file written');
}
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/4] t/lei-up: improve diagnostics for this test
2022-09-30 9:21 [PATCH 0/4] fixes noticed while diagnosing t/lei-up.t Eric Wong
2022-09-30 9:21 ` [PATCH 1/4] tests: favor 3 argument `open' with interopolation Eric Wong
@ 2022-09-30 9:21 ` Eric Wong
2022-09-30 9:21 ` [PATCH 3/4] lei_to_mail: propagate errors to script/lei Eric Wong
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Eric Wong @ 2022-09-30 9:21 UTC (permalink / raw)
To: meta
I'm getting occasional failures for this test on CentOS 7.x (but
not on FreeBSD nor Debian 10/11). I'm not why, yet, so just
improve diagnostics for now.
---
t/lei-up.t | 43 +++++++++++++++++++++++++++----------------
1 file changed, 27 insertions(+), 16 deletions(-)
diff --git a/t/lei-up.t b/t/lei-up.t
index 022ebc05..baed6507 100644
--- a/t/lei-up.t
+++ b/t/lei-up.t
@@ -5,39 +5,50 @@ use strict; use v5.10.1; use PublicInbox::TestCommon;
use IO::Uncompress::Gunzip qw(gunzip $GunzipError);
test_lei(sub {
my ($ro_home, $cfg_path) = setup_public_inboxes;
- my $s = eml_load('t/plack-qp.eml')->as_string;
+ my $home = $ENV{HOME};
+ my $qp = eml_load('t/plack-qp.eml');
+ my $s = $qp->as_string;
lei_ok [qw(import -q -F eml -)], undef, { 0 => \$s, %$lei_opt };
- lei_ok qw(q z:0.. -f mboxcl2 -o), "$ENV{HOME}/a.mbox.gz";
- lei_ok qw(q z:0.. -f mboxcl2 -o), "$ENV{HOME}/b.mbox.gz";
- lei_ok qw(q z:0.. -f mboxcl2 -o), "$ENV{HOME}/a";
- lei_ok qw(q z:0.. -f mboxcl2 -o), "$ENV{HOME}/b";
+ lei_ok qw(q z:0.. -f mboxcl2 -o), "$home/a.mbox.gz";
+ lei_ok qw(q z:0.. -f mboxcl2 -o), "$home/b.mbox.gz";
+ lei_ok qw(q z:0.. -f mboxcl2 -o), "$home/a";
+ lei_ok qw(q z:0.. -f mboxcl2 -o), "$home/b";
+ my $uc;
+ for my $x (qw(a b)) {
+ gunzip("$home/$x.mbox.gz" => \$uc, MultiStream => 1) or
+ xbail "gunzip $GunzipError";
+ ok(index($uc, $qp->body_raw) >= 0,
+ "original mail in $x.mbox.gz");
+ open my $fh, '<', "$home/$x" or xbail $!;
+ $uc = do { local $/; <$fh> } // xbail $!;
+ ok(index($uc, $qp->body_raw) >= 0,
+ "original mail in uncompressed $x");
+ }
lei_ok qw(ls-search);
$s = eml_load('t/utf8.eml')->as_string;
lei_ok [qw(import -q -F eml -)], undef, { 0 => \$s, %$lei_opt };
lei_ok qw(up --all=local);
- open my $fh, '<', "$ENV{HOME}/a.mbox.gz" or xbail "open: $!";
- my $gz = do { local $/; <$fh> };
- my $uc;
- gunzip(\$gz => \$uc, MultiStream => 1) or xbail "gunzip $GunzipError";
- open $fh, '<', "$ENV{HOME}/a" or xbail "open: $!";
+ gunzip("$home/a.mbox.gz" => \$uc, MultiStream => 1) or
+ xbail "gunzip $GunzipError";
+
+ open my $fh, '<', "$home/a" or xbail "open: $!";
my $exp = do { local $/; <$fh> };
is($uc, $exp, 'compressed and uncompressed match (a.gz)');
like($exp, qr/testmessage\@example.com/, '2nd message added');
- open $fh, '<', "$ENV{HOME}/b.mbox.gz" or xbail "open: $!";
- $gz = do { local $/; <$fh> };
undef $uc;
- gunzip(\$gz => \$uc, MultiStream => 1) or xbail "gunzip $GunzipError";
+ gunzip("$home/b.mbox.gz" => \$uc, MultiStream => 1) or
+ xbail "gunzip $GunzipError";
is($uc, $exp, 'compressed and uncompressed match (b.gz)');
- open $fh, '<', "$ENV{HOME}/b" or xbail "open: $!";
+ open $fh, '<', "$home/b" or xbail "open: $!";
$uc = do { local $/; <$fh> };
is($uc, $exp, 'uncompressed both match');
- lei_ok [ qw(up -q), "$ENV{HOME}/b", "--mua=touch $ENV{HOME}/c" ],
+ lei_ok [ qw(up -q), "$home/b", "--mua=touch $home/c" ],
undef, { run_mode => 0 };
- ok(-f "$ENV{HOME}/c", '--mua works with single output');
+ ok(-f "$home/c", '--mua works with single output');
});
done_testing;
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/4] lei_to_mail: propagate errors to script/lei
2022-09-30 9:21 [PATCH 0/4] fixes noticed while diagnosing t/lei-up.t Eric Wong
2022-09-30 9:21 ` [PATCH 1/4] tests: favor 3 argument `open' with interopolation Eric Wong
2022-09-30 9:21 ` [PATCH 2/4] t/lei-up: improve diagnostics for this test Eric Wong
@ 2022-09-30 9:21 ` Eric Wong
2022-09-30 9:21 ` [PATCH 4/4] t/altid_v2: improve test style Eric Wong
2022-09-30 17:20 ` SQLite <3.8.3 was broken on fork (was: fixes noticed while diagnosing t/lei-up.t) Eric Wong
4 siblings, 0 replies; 6+ messages in thread
From: Eric Wong @ 2022-09-30 9:21 UTC (permalink / raw)
To: meta
We need to rely on lei->fail to propagate errors in lei workers
to the script/lei client, otherwise tests and other scripts can
stumble forward with incomplete/incorrect/broken outputs.
This helps me focus on occasional t/lei-up.t failures I see on
CentOS 7.x where OverIdx->adj_counter fails on "lei up --all"...
---
lib/PublicInbox/LeiToMail.pm | 34 ++++++++++++++++++++--------------
1 file changed, 20 insertions(+), 14 deletions(-)
diff --git a/lib/PublicInbox/LeiToMail.pm b/lib/PublicInbox/LeiToMail.pm
index 03cbde3b..b58e2652 100644
--- a/lib/PublicInbox/LeiToMail.pm
+++ b/lib/PublicInbox/LeiToMail.pm
@@ -132,19 +132,22 @@ sub eml2mboxcl2 {
}
sub git_to_mail { # git->cat_async callback
- my ($bref, $oid, $type, $size, $arg) = @_;
+ my ($bref, $oid, $type, $size, $smsg) = @_;
+ my $self = delete $smsg->{l2m} // die "BUG: no l2m";
$type // return; # called by git->async_abort
- my ($write_cb, $smsg) = @$arg;
- if ($type eq 'missing' && $smsg->{-lms_rw}) {
- if ($bref = $smsg->{-lms_rw}->local_blob($oid, 1)) {
+ eval {
+ if ($type eq 'missing' &&
+ ($bref = $self->{-lms_rw}->local_blob($oid, 1))) {
$type = 'blob';
$size = length($$bref);
}
- }
- return warn("W: $oid is $type (!= blob)\n") if $type ne 'blob';
- return warn("E: $oid is empty\n") unless $size;
- die "BUG: expected=$smsg->{blob} got=$oid" if $smsg->{blob} ne $oid;
- $write_cb->($bref, $smsg);
+ $type eq 'blob' or return $self->{lei}->child_error(1,
+ "W: $oid is $type (!= blob)");
+ $size or return $self->{lei}->child_error(1,"E: $oid is empty");
+ $smsg->{blob} eq $oid or die "BUG: expected=$smsg->{blob}";
+ $self->{wcb}->($bref, $smsg);
+ };
+ $self->{lei}->fail("$@ (oid=$oid)") if $@;
}
sub reap_compress { # dwaitpid callback
@@ -790,19 +793,22 @@ sub poke_dst {
sub write_mail { # via ->wq_io_do
my ($self, $smsg, $eml) = @_;
- return $self->{wcb}->(undef, $smsg, $eml) if $eml;
- $smsg->{-lms_rw} = $self->{-lms_rw};
- $self->{git}->cat_async($smsg->{blob}, \&git_to_mail,
- [$self->{wcb}, $smsg]);
+ if ($eml) {
+ eval { $self->{wcb}->(undef, $smsg, $eml) };
+ $self->{lei}->fail("blob=$smsg->{blob} $@") if $@;
+ } else {
+ $smsg->{l2m} = $self;
+ $self->{git}->cat_async($smsg->{blob}, \&git_to_mail, $smsg);
+ }
}
sub wq_atexit_child {
my ($self) = @_;
local $PublicInbox::DS::in_loop = 0; # waitpid synchronously
my $lei = $self->{lei};
- delete $self->{wcb};
$lei->{ale}->git->async_wait_all;
my ($nr_w, $nr_s) = delete(@$lei{qw(-nr_write -nr_seen)});
+ delete $self->{wcb};
$nr_s or return;
return if $lei->{early_mua} || !$lei->{-progress} || !$lei->{pkt_op_p};
$lei->{pkt_op_p}->pkt_do('l2m_progress', $nr_w, $nr_s);
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 4/4] t/altid_v2: improve test style
2022-09-30 9:21 [PATCH 0/4] fixes noticed while diagnosing t/lei-up.t Eric Wong
` (2 preceding siblings ...)
2022-09-30 9:21 ` [PATCH 3/4] lei_to_mail: propagate errors to script/lei Eric Wong
@ 2022-09-30 9:21 ` Eric Wong
2022-09-30 17:20 ` SQLite <3.8.3 was broken on fork (was: fixes noticed while diagnosing t/lei-up.t) Eric Wong
4 siblings, 0 replies; 6+ messages in thread
From: Eric Wong @ 2022-09-30 9:21 UTC (permalink / raw)
To: meta
Favor `is' for equality checks since it reports differences,
and `xbail' over `BAIL_OUT' since it's easier-to-type w/o caps
and more powerful.
These are just things noticed while I was looking at another
odd failure on CentOS 7.x with this test, but I suspect it
was a transient failure caused by running the test suite
from multiple terminals in parallel.
---
t/altid_v2.t | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/t/altid_v2.t b/t/altid_v2.t
index 281a09d5..c62252c1 100644
--- a/t/altid_v2.t
+++ b/t/altid_v2.t
@@ -14,9 +14,9 @@ my $ibx = create_inbox 'v2', version => 2, indexlevel => 'medium',
altid => $altid, sub {
my ($im, $ibx) = @_;
my $mm = PublicInbox::Msgmap->new_file("$ibx->{inboxdir}/$another", 2);
- $mm->mid_set(1234, 'a@example.com') == 1 or BAIL_OUT 'mid_set once';
- ok(0 == $mm->mid_set(1234, 'a@example.com'), 'mid_set not idempotent');
- ok(0 == $mm->mid_set(1, 'a@example.com'), 'mid_set fails with dup MID');
+ is($mm->mid_set(1234, 'a@example.com'), 1, 'mid_set') or xbail 'once';
+ is($mm->mid_set(1234, 'a@example.com')+0, 0, 'mid_set not idempotent');
+ is($mm->mid_set(1, 'a@example.com')+0, 0, 'mid_set fails with dup MID');
$im->add(PublicInbox::Eml->new(<<'EOF')) or BAIL_OUT;
From: a@example.com
To: b@example.com
@@ -27,8 +27,8 @@ hello world gmane:666
EOF
};
my $mm = PublicInbox::Msgmap->new_file("$ibx->{inboxdir}/$another", 2);
-ok(0 == $mm->mid_set(1234, 'a@example.com'), 'mid_set not idempotent');
-ok(0 == $mm->mid_set(1, 'a@example.com'), 'mid_set fails with dup MID');
+is($mm->mid_set(1234, 'a@example.com') + 0, 0, 'mid_set not idempotent');
+is($mm->mid_set(1, 'a@example.com') + 0, 0, 'mid_set fails with dup MID');
my $mset = $ibx->search->mset('gmane:1234');
my $msgs = $ibx->search->mset_to_smsg($ibx, $mset);
$msgs = [ map { $_->{mid} } @$msgs ];
^ permalink raw reply related [flat|nested] 6+ messages in thread
* SQLite <3.8.3 was broken on fork (was: fixes noticed while diagnosing t/lei-up.t)
2022-09-30 9:21 [PATCH 0/4] fixes noticed while diagnosing t/lei-up.t Eric Wong
` (3 preceding siblings ...)
2022-09-30 9:21 ` [PATCH 4/4] t/altid_v2: improve test style Eric Wong
@ 2022-09-30 17:20 ` Eric Wong
4 siblings, 0 replies; 6+ messages in thread
From: Eric Wong @ 2022-09-30 17:20 UTC (permalink / raw)
To: meta
Eric Wong <e@80x24.org> wrote:
> I'm still trying to figure out why OverIdx->adj_counter (via
> next_tid) in the LeiSavedSearch dedupe check occasionally fails
> under CentOS 7.x (but not other systems).
CentOS 7.x ships SQLite 3.7.17, and SQLite prior to 3.8.3 (2014-02-03)
didn't reset its PRNG for generating temporary filenames upon fork.
I can't find a way reset that PRNG via DBD::SQLite, either...
One possible nasty thing I could do is reset TMPDIR upon fork,
but that may leave bugs lingering, too :<
Here's the debugging patch I used to strace what was going on:
(I may just make lei_ok bail out on error...)
diff --git a/lib/PublicInbox/OverIdx.pm b/lib/PublicInbox/OverIdx.pm
index e7c96e14..e1af910c 100644
--- a/lib/PublicInbox/OverIdx.pm
+++ b/lib/PublicInbox/OverIdx.pm
@@ -51,10 +51,22 @@ SELECT val FROM counter WHERE key = ? LIMIT 1
sub adj_counter ($$$) {
my ($self, $key, $op) = @_;
my $dbh = $self->{dbh};
+use PublicInbox::Spawn qw(spawn);
+my $trace = "/tmp/$$.strace";
+open my $err, '>>', $trace;
+my $pid = spawn([qw(strace -f -s4096 -v -p), $$], undef, { 2 => $err });
+select undef, undef, undef, 0.1;
+
my $sth = $dbh->prepare_cached(<<"");
UPDATE counter SET val = val $op 1 WHERE key = ?
- $sth->execute($key);
+ eval { $sth->execute($key) };
+my $err = $@;
+syswrite(STDERR, "$$ $err\n") if $err;
+kill('TERM', $pid);
+waitpid($pid, 0);
+die $err if $err;
+#unlink($trace);
get_counter($dbh, $key);
}
diff --git a/t/lei-up.t b/t/lei-up.t
index baed6507..9c65a243 100644
--- a/t/lei-up.t
+++ b/t/lei-up.t
@@ -27,7 +27,7 @@ test_lei(sub {
lei_ok qw(ls-search);
$s = eml_load('t/utf8.eml')->as_string;
lei_ok [qw(import -q -F eml -)], undef, { 0 => \$s, %$lei_opt };
- lei_ok qw(up --all=local);
+ lei_ok qw(up --all=local) or xbail "lei up --all=local failed $?";
gunzip("$home/a.mbox.gz" => \$uc, MultiStream => 1) or
xbail "gunzip $GunzipError";
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-09-30 17:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-09-30 9:21 [PATCH 0/4] fixes noticed while diagnosing t/lei-up.t Eric Wong
2022-09-30 9:21 ` [PATCH 1/4] tests: favor 3 argument `open' with interopolation Eric Wong
2022-09-30 9:21 ` [PATCH 2/4] t/lei-up: improve diagnostics for this test Eric Wong
2022-09-30 9:21 ` [PATCH 3/4] lei_to_mail: propagate errors to script/lei Eric Wong
2022-09-30 9:21 ` [PATCH 4/4] t/altid_v2: improve test style Eric Wong
2022-09-30 17:20 ` SQLite <3.8.3 was broken on fork (was: fixes noticed while diagnosing t/lei-up.t) 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).