Re: Trying to add more tests to gistbuild.c - Mailing list pgsql-hackers

From Aleksander Alekseev
Subject Re: Trying to add more tests to gistbuild.c
Date
Msg-id CAJ7c6TNT8ZE0_eGRSad4zJ8O-A5qeVmpONMwtF543RKGeBGyjg@mail.gmail.com
Whole thread Raw
In response to Re: Trying to add more tests to gistbuild.c  (Matheus Alcantara <mths.dev@pm.me>)
Responses Re: Trying to add more tests to gistbuild.c
List pgsql-hackers
Hi Matheus,

Many thanks for hacking on increasing the code coverage! I noticed
that this patch was stuck in "Needs Review" state for some time and
decided to take a look.

> With these new tests the coverage went from 45.3% to 85.5%, but I have some doubts:
> - Does this test make sense?
> - Would there be a way to validate that the buffering was done correctly?
> - Is this test necessary?

I can confirm that the coverage improved as stated.

Personally I believe this change makes perfect sense. Although this is
arguably not an ideal test for gistInitBuffering(), writing proper
tests for `static` procedures is generally not an easy task. Executing
the code at least once in order to make sure that it doesn't crash,
doesn't throw errors and doesn't violate any Asserts() is better than
doing nothing.

Here is a slightly modified patch with added commit message. Please
note that patches created with `git format-patch` are generally
preferable than patches created with `git diff`.

I'm going to change the status of this patch to "Ready for Committer"
in a short time unless anyone has a second opinion.

-- 
Best regards,
Aleksander Alekseev

Attachment

pgsql-hackers by date:

Previous
From: Amit Langote
Date:
Subject: Re: Skip partition tuple routing with constant partition key
Next
From: Andrew Dunstan
Date:
Subject: Re: fairywren hung in pg_basebackup tests