On Mon, Mar 19, 2018 at 7:57 PM, Masahiko Sawada <sawada.m...@gmail.com> wrote: > On Mon, Mar 19, 2018 at 12:50 PM, Masahiko Sawada <sawada.m...@gmail.com> > wrote: >> On Fri, Mar 16, 2018 at 10:24 AM, Alvaro Herrera >> <alvhe...@alvh.no-ip.org> wrote: >>> Masahiko Sawada wrote: >>>> On Thu, Mar 15, 2018 at 9:41 AM, Alvaro Herrera <alvhe...@alvh.no-ip.org> >>>> wrote: >>> >>>> > I think this is a worthwhile test, but IMO it should be improved a bit >>>> > before we include it. Also, we can come up with a better name for the >>>> > test surely, not just refer to this particular bug. Maybe "typemap". >>>> >>>> It might be useful if we have the facility of TAP test to check the >>>> log message and regexp-match the message to the expected string. >>> >>> Something similar to PostgresNode::issues_sql_like() perhaps? >>> >> >> Yeah, I didn't know that but I think it's a good idea. Unlike >> issues_sql_like() we don't issue anything to the subscriber. So maybe >> we need truncate logfile before insertion and verify logfile of >> particular period. The test code would be like follows. >> >> $node_subscriber->safe_psql('postgres', 'CREATE SUBSCRIPTION..."); >> truncate $node_subscriber->logfile, 0; >> $node_publisher->safe_psql('postgres', 'INSERT .. ') >> my $log = TestLib::slurp_file($node_subscriber->logfile); >> >> # Verify logs >> like($log, qr/processing remote data for replication target relation >> "public.test" column "b", remote type dummyint, local type dummyint/, >> 'callback function of datatype conversion1'); >> like($log, qr/processing remote data for replication target relation >> "public.test" column "a", remote type dummytext, local type >> dummytext/, 'callback function of datatype conversion2'); >> >> Thoughts? >> > > Attached an updated test patch added the above test(0002 patch). Since > for this test case it's enough to use existing test functions I didn't > create new test functions. Also I found that the local data type name > in log for data type conversion isn't qualified whereas the remote > data type is always qualified. Attached 0001 patch fixes that. >
The original issue has been fixed and this entry on CommitFest has been marked as "Committed" but there are still works for improving testing. Perhaps I should register a new entry of remaining patches to next CommitFest. Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center