fix: [Postgres] reset binds when replace() method is called multiple times in the context

[michalsn]

Description
Fixes #6726

Code provided by kenjis. Thanks!

Checklist:

• Securely signed commits
• Component(s) with PHPDoc blocks, only if necessary or adds value
• Unit testing, with >80% coverage
• User guide updated
• Conforms to style guide

[sclubricants]

Can we consolidate this test with testReplaceWithNoMatchingData() and testReplaceWithMatchingData()?

It seems all three tests could be accomplished in a single test.

[kenjis]

I disagree. It is bad practice.

testReplaceWithNoMatchingData() is test for replace with no matching data.
testReplaceWithMatchingData() is test for replace with matching data.
testReplaceTwice() is test for replace twice.
They all differ in the intent of the test.

[sclubricants]

If replace works twice then it also works once.

testReplaceWithMatchingData() is test for replace with matching data.

Not sure what this is testing. It really doesn't matter if there is matching data or not. Whatever data is there is deleted and whatever is defined is replaced. Its not like it senses no changes and leaves it alone. If it works with mismatched data then is works without it.

I think we need to keep tests minimized where possible so that we don't have an hour of tests to run everytime. Yes they should be thorough though.

[michalsn]

You both have a good point.

I think we are used to making tests based on the behavior of the methods. In addition, if there is a bug, we always make a separate test for it (it's some sort of a rule).

Even if merging everything into one test would save us 1 sec, I am not sure if it would be worth it in the long run. Readability of tests is important - we should not test 3 different things in one test.

I added a reference to the issue above the test and I'm gonna leave it as it is.

[kenjis]

Yes, slow tests are surely another issue.

Not sure what this is testing. It really doesn't matter if there is matching data or not.

From a testing perspective, it really matters.
If you don't have the test, a needed test case is missing.

Whatever data is there is deleted and whatever is defined is replaced.

We test to see if these data can really be replaced.
All those cases (with matching data or without matching data) should be tested.

If replace works twice then it also works once.

It probably does, but not for sure. It depends on the production code.
But in this case, I also believe it does.

[kenjis]

LGTM!