user/dev discussion of public-inbox itself
 help / color / mirror / code / Atom feed
Search results ordered by [date|relevance]  view[summary|nested|Atom feed]
thread overview below | download mbox.gz: |
* [PATCH 03/13] t/v2reindex.t: Isolate the test cases more
  2018-08-01 16:41  7% [PATCH 00/13]: Incremental index fixes Eric W. Biederman
@ 2018-08-01 16:43  6% ` Eric W. Biederman
  0 siblings, 0 replies; 2+ results
From: Eric W. Biederman @ 2018-08-01 16:43 UTC (permalink / raw)
  To: Eric Wong; +Cc: meta, Eric W. Biederman

While inspecting the tests I realized that because we have been
reusing variables there can be a memory between one test case and
another.  Add scopes and local variables to prevent an unintended
memory between one test cases.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 t/v2reindex.t | 85 ++++++++++++++++++++++++++++++++-------------------
 1 file changed, 54 insertions(+), 31 deletions(-)

diff --git a/t/v2reindex.t b/t/v2reindex.t
index 1543309cf1ee..4d06c6ce6ea6 100644
--- a/t/v2reindex.t
+++ b/t/v2reindex.t
@@ -21,7 +21,6 @@ my $ibx_config = {
 	-primary_address => 'test@example.com',
 	indexlevel => 'full',
 };
-my $ibx = PublicInbox::Inbox->new($ibx_config);
 my $mime = PublicInbox::MIME->create(
 	header => [
 		From => 'a@example.com',
@@ -32,39 +31,57 @@ my $mime = PublicInbox::MIME->create(
 	body => "hello world\n",
 );
 local $ENV{NPROC} = 2;
-my $im = PublicInbox::V2Writable->new($ibx, 1);
-foreach my $i (1..10) {
-	$mime->header_set('Message-Id', "<$i\@example.com>");
-	ok($im->add($mime), "message $i added");
-	if ($i == 4) {
+my $minmax;
+{
+	my %config = %$ibx_config;
+	my $ibx = PublicInbox::Inbox->new(\%config);
+	my $im = PublicInbox::V2Writable->new($ibx, 1);
+	foreach my $i (1..10) {
+		$mime->header_set('Message-Id', "<$i\@example.com>");
+		ok($im->add($mime), "message $i added");
+		if ($i == 4) {
+			$im->remove($mime);
+		}
+	}
+
+	if ('test remove later') {
+		$mime->header_set('Message-Id', "<5\@example.com>");
 		$im->remove($mime);
 	}
-}
 
-if ('test remove later') {
-	$mime->header_set('Message-Id', "<5\@example.com>");
-	$im->remove($mime);
+	$im->done;
+	$minmax = [ $ibx->mm->minmax ];
+	ok(defined $minmax->[0] && defined $minmax->[1], 'minmax defined');
+	is_deeply($minmax, [ 1, 10 ], 'minmax as expected');
 }
 
-$im->done;
-my $minmax = [ $ibx->mm->minmax ];
-ok(defined $minmax->[0] && defined $minmax->[1], 'minmax defined');
-is_deeply($minmax, [ 1, 10 ], 'minmax as expected');
+{
+	my %config = %$ibx_config;
+	my $ibx = PublicInbox::Inbox->new(\%config);
+	my $im = PublicInbox::V2Writable->new($ibx, 1);
+	eval { $im->index_sync({reindex => 1}) };
+	is($@, '', 'no error from reindexing');
+	$im->done;
 
-eval { $im->index_sync({reindex => 1}) };
-is($@, '', 'no error from reindexing');
-$im->done;
+	delete $ibx->{mm};
+	is_deeply([ $ibx->mm->minmax ], $minmax, 'minmax unchanged');
+}
 
 my $xap = "$mainrepo/xap".PublicInbox::Search::SCHEMA_VERSION();
 remove_tree($xap);
 ok(!-d $xap, 'Xapian directories removed');
-eval { $im->index_sync({reindex => 1}) };
-is($@, '', 'no error from reindexing');
-$im->done;
-ok(-d $xap, 'Xapian directories recreated');
+{
+	my %config = %$ibx_config;
+	my $ibx = PublicInbox::Inbox->new(\%config);
+	my $im = PublicInbox::V2Writable->new($ibx, 1);
+	eval { $im->index_sync({reindex => 1}) };
+	is($@, '', 'no error from reindexing');
+	$im->done;
+	ok(-d $xap, 'Xapian directories recreated');
 
-delete $ibx->{mm};
-is_deeply([ $ibx->mm->minmax ], $minmax, 'minmax unchanged');
+	delete $ibx->{mm};
+	is_deeply([ $ibx->mm->minmax ], $minmax, 'minmax unchanged');
+}
 
 ok(unlink "$mainrepo/msgmap.sqlite3", 'remove msgmap');
 remove_tree($xap);
@@ -72,6 +89,9 @@ ok(!-d $xap, 'Xapian directories removed again');
 {
 	my @warn;
 	local $SIG{__WARN__} = sub { push @warn, @_ };
+	my %config = %$ibx_config;
+	my $ibx = PublicInbox::Inbox->new(\%config);
+	my $im = PublicInbox::V2Writable->new($ibx, 1);
 	eval { $im->index_sync({reindex => 1}) };
 	is($@, '', 'no error from reindexing without msgmap');
 	is(scalar(@warn), 0, 'no warnings from reindexing');
@@ -88,6 +108,9 @@ ok(!-d $xap, 'Xapian directories removed again');
 {
 	my @warn;
 	local $SIG{__WARN__} = sub { push @warn, @_ };
+	my %config = %$ibx_config;
+	my $ibx = PublicInbox::Inbox->new(\%config);
+	my $im = PublicInbox::V2Writable->new($ibx, 1);
 	eval { $im->index_sync({reindex => 1}) };
 	is($@, '', 'no error from reindexing without msgmap');
 	is_deeply(\@warn, [], 'no warnings');
@@ -103,13 +126,13 @@ ok(!-d $xap, 'Xapian directories removed again');
 ok(unlink "$mainrepo/msgmap.sqlite3", 'remove msgmap');
 remove_tree($xap);
 ok(!-d $xap, 'Xapian directories removed again');
-
-$ibx_config->{indexlevel} = 'medium';
-$ibx = PublicInbox::Inbox->new($ibx_config);
-$im = PublicInbox::V2Writable->new($ibx);
 {
 	my @warn;
 	local $SIG{__WARN__} = sub { push @warn, @_ };
+	my %config = %$ibx_config;
+	$config{indexlevel} = 'medium';
+	my $ibx = PublicInbox::Inbox->new(\%config);
+	my $im = PublicInbox::V2Writable->new($ibx);
 	eval { $im->index_sync({reindex => 1}) };
 	is($@, '', 'no error from reindexing without msgmap');
 	is_deeply(\@warn, [], 'no warnings');
@@ -135,13 +158,13 @@ $im = PublicInbox::V2Writable->new($ibx);
 ok(unlink "$mainrepo/msgmap.sqlite3", 'remove msgmap');
 remove_tree($xap);
 ok(!-d $xap, 'Xapian directories removed again');
-
-$ibx_config->{indexlevel} = 'basic';
-$ibx = PublicInbox::Inbox->new($ibx_config);
-$im = PublicInbox::V2Writable->new($ibx);
 {
 	my @warn;
 	local $SIG{__WARN__} = sub { push @warn, @_ };
+	my %config = %$ibx_config;
+	$config{indexlevel} = 'basic';
+	my $ibx = PublicInbox::Inbox->new(\%config);
+	my $im = PublicInbox::V2Writable->new($ibx);
 	eval { $im->index_sync({reindex => 1}) };
 	is($@, '', 'no error from reindexing without msgmap');
 	is_deeply(\@warn, [], 'no warnings');
-- 
2.17.1


^ permalink raw reply related	[relevance 6%]

* [PATCH 00/13]: Incremental index fixes
@ 2018-08-01 16:41  7% Eric W. Biederman
  2018-08-01 16:43  6% ` [PATCH 03/13] t/v2reindex.t: Isolate the test cases more Eric W. Biederman
  0 siblings, 1 reply; 2+ results
From: Eric W. Biederman @ 2018-08-01 16:41 UTC (permalink / raw)
  To: Eric Wong; +Cc: meta


Eric,

Recently I noticed some weird indexing on one of my mailboxes.  So I dug
in and made the index_sync tests more robust and found several corner
cases where the current code can be problematic.  What follows are the
test cases and fixes for those cases.

The big one is when a message with the highest message number is
deleted.  This can cause message numbers to be reassigned so I
explicitly track the highest message number assigned.

Very confusing when reading mail there is a case in SearchIdx
that has been assigning message numbers in the wrong order.  Depending
on your time range that can be very confusing as old messages show up
before newer ones.

Finally in v2 deleted messages have not been being deleted from the
msgmap.  Which while great for keeping message numbers from going
backwards it means things still show up that shouldn't.

Eric W. Biederman (13):
      Import.pm: Don't assume {in} and {out} always exist
      t/v1reindex.t: Isolate the test cases
      t/v2reindex.t: Isolate the test cases more
      t/v[12]reindex.t: Place expected second in Xapian tests
      t/v[12]reindex.t: Test that the resulting msgmap is as expected
      t/v[12]reindex.t: Test incremental indexing works
      SearchIdx.pm: Always assign numbers backwards during incremental indexing
      Msgmap.pm: Track the largest value of num ever assigned
      t/v[12]reindex.t Verify num_highwater
      t/v[12]reindex.t: Verify the num highwater is as expected
      SearchIdx,V2Writeable: Update num_highwater on optimized deletes
      V2Writeable.pm: Ensure that a found message number is in the msgmap
      V2Writeable.pm: In unindex_oid delete the message from msgmap

 lib/PublicInbox/Import.pm     |   1 +
 lib/PublicInbox/Msgmap.pm     |  23 ++-
 lib/PublicInbox/SearchIdx.pm  |  30 ++--
 lib/PublicInbox/V2Writable.pm |  20 ++-
 t/v1reindex.t                 | 359 ++++++++++++++++++++++++++++++++++++------
 t/v2reindex.t                 | 335 +++++++++++++++++++++++++++++++++++----
 6 files changed, 666 insertions(+), 102 deletions(-)


Eric



^ permalink raw reply	[relevance 7%]

Results 1-2 of 2 | reverse | options above
-- pct% links below jump to the message on this page, permalinks otherwise --
2018-08-01 16:41  7% [PATCH 00/13]: Incremental index fixes Eric W. Biederman
2018-08-01 16:43  6% ` [PATCH 03/13] t/v2reindex.t: Isolate the test cases more Eric W. Biederman

Code repositories for project(s) associated with this public inbox

	https://80x24.org/public-inbox.git

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).