1/3 is the most important; more watch tweaks coming... Eric Wong (3): watch: flush changes to inbox before updating IMAPTracker imaptracker: update_last: simplify callers tests: check-run: show skipped tests lib/PublicInbox/IMAPTracker.pm | 5 +++-- lib/PublicInbox/WatchMaildir.pm | 4 ++-- t/run.perl | 22 ++++++++++++++++++++-- 3 files changed, 25 insertions(+), 6 deletions(-)
Data needs to hit inboxes, first. Otherwise it's possible to skip messages in case git-fast-import is killed before it sees "done\n". Now, -watch will just waste a little bandwidth in re-downloading a seen message if it's interrupted immediately before updating IMAPTracker. --- lib/PublicInbox/WatchMaildir.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/PublicInbox/WatchMaildir.pm b/lib/PublicInbox/WatchMaildir.pm index 4ae400f7..78aec8a2 100644 --- a/lib/PublicInbox/WatchMaildir.pm +++ b/lib/PublicInbox/WatchMaildir.pm @@ -918,8 +918,8 @@ sub nntp_fetch_all ($$$) { } $last_art = $art; } - $itrk->update_last(0, $last_art) if defined $last_art; _done_for_now($self); + $itrk->update_last(0, $last_art) if defined $last_art; $err; }
By making it a no-op if last_uid is not defined. This isn't a hot code path, so the extra method dispatch isn't an issue. It'll save some indentation/wrapping in future commits. --- lib/PublicInbox/IMAPTracker.pm | 5 +++-- lib/PublicInbox/WatchMaildir.pm | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/PublicInbox/IMAPTracker.pm b/lib/PublicInbox/IMAPTracker.pm index 92f21584..be9caf76 100644 --- a/lib/PublicInbox/IMAPTracker.pm +++ b/lib/PublicInbox/IMAPTracker.pm @@ -49,13 +49,14 @@ SELECT uid_validity, uid FROM imap_last WHERE url = ? } sub update_last ($$$) { - my ($self, $validity, $last) = @_; + my ($self, $validity, $last_uid) = @_; + return unless defined $last_uid; my $sth = $self->{dbh}->prepare_cached(<<''); INSERT OR REPLACE INTO imap_last (url, uid_validity, uid) VALUES (?, ?, ?) $self->lock_acquire; - my $rv = $sth->execute($self->{url}, $validity, $last); + my $rv = $sth->execute($self->{url}, $validity, $last_uid); $self->lock_release; $rv; } diff --git a/lib/PublicInbox/WatchMaildir.pm b/lib/PublicInbox/WatchMaildir.pm index 78aec8a2..a227a6fd 100644 --- a/lib/PublicInbox/WatchMaildir.pm +++ b/lib/PublicInbox/WatchMaildir.pm @@ -494,7 +494,7 @@ sub imap_fetch_all ($$$) { last if $self->{quit}; } _done_for_now($self); - $itrk->update_last($r_uidval, $last_uid) if defined $last_uid; + $itrk->update_last($r_uidval, $last_uid); } until ($err || $self->{quit}); $err; } @@ -919,7 +919,7 @@ sub nntp_fetch_all ($$$) { $last_art = $art; } _done_for_now($self); - $itrk->update_last(0, $last_art) if defined $last_art; + $itrk->update_last(0, $last_art); $err; }
We'll deduplicate redundant lines and show counts of skipped tests to ensure it's easy to notice if something is unexpectedly skipped. --- t/run.perl | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/t/run.perl b/t/run.perl index b1a0d2fe..e3e3e075 100755 --- a/t/run.perl +++ b/t/run.perl @@ -15,6 +15,7 @@ use PublicInbox::TestCommon; use Cwd qw(getcwd); use Getopt::Long qw(:config gnu_getopt no_ignore_case auto_abbrev); use Errno qw(EINTR); +use Fcntl qw(:seek); use POSIX qw(_POSIX_PIPE_BUF WNOHANG); my $jobs = 1; my $repeat = 1; @@ -65,14 +66,31 @@ sub test_status () { if ($log_suffix ne '') { my $log = $worker_test; $log =~ s/\.t\z/$log_suffix/; + my $skip = ''; if (open my $fh, '<', $log) { my @not_ok = grep(!/^(?:ok |[ \t]*#)/ms, <$fh>); pop @not_ok if $not_ok[-1] =~ /^[0-9]+\.\.[0-9]+$/; - print OLDERR map { "# $log: $_" } @not_ok; + my $pfx = "# $log: "; + print OLDERR map { $pfx.$_ } @not_ok; + seek($fh, 0, SEEK_SET) or die "seek: $!"; + + # show unique skip texts and the number of times + # each text was skipped + local $/; + my @sk = (<$fh> =~ m/^ok [0-9]+ (# skip [^\n]+)/mgs); + if (@sk) { + my %nr; + $nr{$_}++ for @sk; + for (@sk) { + my $n = delete $nr{$_} or next; + print OLDERR "$pfx$_ ($n)\n"; + } + $skip = ' # total skipped: '.scalar(@sk); + } } else { print OLDERR "could not open: $log: $!\n"; } - print OLDOUT "$status $worker_test\n"; + print OLDOUT "$status $worker_test$skip\n"; } }
Eric Wong <e@yhbt.net> wrote: > + my %nr; > + $nr{$_}++ for @sk; > + for (@sk) { > + my $n = delete $nr{$_} or next; > + print OLDERR "$pfx$_ ($n)\n"; > + } That's longer than it needs to be, we can grep here, too. diff --git a/t/run.perl b/t/run.perl index e3e3e075..0ba5e044 100755 --- a/t/run.perl +++ b/t/run.perl @@ -80,11 +80,8 @@ sub test_status () { my @sk = (<$fh> =~ m/^ok [0-9]+ (# skip [^\n]+)/mgs); if (@sk) { my %nr; - $nr{$_}++ for @sk; - for (@sk) { - my $n = delete $nr{$_} or next; - print OLDERR "$pfx$_ ($n)\n"; - } + my @err = grep { !$nr{$_}++ } @sk; + print OLDERR "$pfx$_ ($nr{$_})\n" for @err; $skip = ' # total skipped: '.scalar(@sk); } } else {