Feedback-ID: ie0a040ee:Fastmail
Message-ID: <86ebc1d3-b1fc-4ad3-a976-2eb79d9bd424@eisentraut.org>
Date: Sun, 12 May 2024 13:46:29 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Add TAP tests for backtrace functionality (was Re: Add test
 module for verifying backtrace functionality)
To: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
References: <CALj2ACXsPaQmmTx4vT1LwhyU-oFsnoZn-xtq3=3kMDt0O1vEug@mail.gmail.com>
 <CALj2ACXmJqpFU8KH8J-RG67FO7YDXBbQVFicU9V29kd4ihE4EA@mail.gmail.com>
 <CALj2ACWv3NAbZgdPsZJ+47J202Zb_q0sNPHLy0-_rebB5okiBA@mail.gmail.com>
Content-Language: en-US
From: Peter Eisentraut <peter@eisentraut.org>
In-Reply-To: <CALj2ACWv3NAbZgdPsZJ+47J202Zb_q0sNPHLy0-_rebB5okiBA@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: <https://www.postgresql.org/message-id/86ebc1d3-b1fc-4ad3-a976-2eb79d9bd424%40eisentraut.org>
Precedence: bulk

On 16.03.24 05:25, Bharath Rupireddy wrote:
>>> Postgres has a good amount of code for dealing with backtraces - two
>>> GUCs backtrace_functions and backtrace_on_internal_error,
>>> errbacktrace; all of which use core function set_backtrace from
>>> elog.c. I've not seen this code being tested at all, see code coverage
>>> report - https://coverage.postgresql.org/src/backend/utils/error/elog.c.gcov.html.
>>>
>>> I think adding a simple test module (containing no .c files) with only
>>> TAP tests will help cover this code. I ended up having it as a
>>> separate module under src/test/modules/test_backtrace as I was not
>>> able to find an existing TAP file in src/test to add these tests.  I'm
>>> able to verify the backtrace related code with the attached patch
>>> consistently. The TAP tests rely on the fact that the server emits
>>> text "BACKTRACE: " to server logs before logging the backtrace, and
>>> the backtrace contains the function name in which the error occurs.
>>> I've turned off query statement logging (set log_statement = none,
>>> log_min_error_statement = fatal) so that the tests get to see the
>>> functions only in the backtrace. Although the CF bot is happy with the
>>> attached patch https://github.com/BRupireddy2/postgres/tree/add_test_module_for_bcktrace_functionality_v1,
>>> there might be some more flakiness to it.
>>>
>>> Thoughts?
>>
>> Ran pgperltidy on the new TAP test file added. Please see the attached v2 patch.
> 
> I've now moved the new TAP test file to src/test/modules/test_misc/t
> as opposed to a new test module to keep it simple. I was not sure why
> I hadn't done that in the first place.

Note that backtrace_on_internal_error has been removed, so this patch 
will need to be adjusted for that.

I suggest you consider joining forces with thread [0] where a 
replacement for backtrace_on_internal_error would be discussed.  Having 
some test coverage for whatever is being developed there might be useful.


[0]: 
https://www.postgresql.org/message-id/flat/CAGECzQTpdujCEt2SH4DBwRLoDq4HJArGDaxJSsWX0G=tNnzaVA@mail.gmail.com