Re: Better testing coverage and unified coding for plpgsql loops

2017-12-30 22:46 GMT+01:00 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:

> While I've been fooling around with plpgsql, I've been paying close
> attention to code coverage reports to make sure that the regression tests
> exercise all that new code. It started to bug me that there were some
> serious gaps in the test coverage for existing code in pl_exec.c.
> One thing I noticed in particular was that coverage for the
> PLPGSQL_RC_EXIT/PLPGSQL_RC_RETURN/PLPGSQL_RC_CONTINUE management code
> in the various looping statements was nearly nonexistent, and coverage
> for integer FOR loops was pretty awful too (eg, not a single BY clause
> in the whole test corpus :-(). So I said to myself "let's fix that",
> and wrote up a new regression test file plpgsql_control.sql with a
> charter to test plpgsql's control structures. I moved a few existing
> tests that seemed to fall into that charter into the new file, and
> added tests until I was happier with the state of the coverage report.
> The result is attached as plpgsql-better-code-coverage.patch.
>
> However, while I was doing that, it seemed like the tests I was adding
> were mighty repetitive, as many of them were just exactly the same thing
> adjusted for a different kind of loop statement. And so I began to wonder
> why it was that we had five copies of the RC_FOO management logic, no two
> quite alike. If we only had *one* copy then it would not seem necessary
> to have such duplicative test cases for it. A bit of hacking later, and
> I had the management logic expressed as a macro, with only one copy for
> all five kinds of loop. I verified it still passes the previous set of
> tests and then removed the ones that seemed redundant, yielding
> plpgsql-unify-loop-rc-code.patch below. So what I propose actually
> committing is the combination of these two patches.
>
> Anyone feel like reviewing this, or should I just push it? It seems
> pretty noncontroversial to me, but maybe I'm wrong about that.
>

I don't think any issue there. This macro is little bit massive, but
similar is used elsewhere.

+1

Pavel

> regards, tom lane
>
>