Make coherent_sets_w_instances timing-independent for non-GROUP presentation

[sqt]

Motivation

Currently, coherent_sets_w_instances (Test_CoherentSets_10/11/12/19/20/21) asserts exactly 36 samples arrive between consecutive polls. Unfortunately, that's a property of how the subscriber's take() period happens to line up with the publisher's write() period, not a property of the PRESENTATION coherent_access contract itself. For INSTANCE_PRESENTATION and TOPIC_PRESENTATION specifically, coherent_access is scoped per DataWriter, so the three topics' coherent sets are never guaranteed to land in the same read cycle. Only the GROUP_PRESENTATION contract promises that. The result of assuming 36 arrive between polls is a high false-failure rate on CS10/11/19/20 whenever a poll catches a partial-but-complete set, independent of any implementation's correctness.

Description of changes

In order to address this, coherent_sets_w_instances now takes an expected_total parameter (default 36, unchanged) and checks the per-cycle sample count as a multiple of it rather than requiring exact equality. A new coherent_sets_w_instances_non_group passes expected_total=12 (one DataWriter's own set: 4 instances × 3 samples) and is used by CS10/11/19/20; CS12/21 (GROUP_PRESENTATION) are unchanged.

[ClarkTucker]

The access pattern that the spec requires to get data as a coherent set is:

Subscriber.begin_access()
Subscriber.get_datareaders()
for each returned dr: dr.read()/take()
Subscriber.end_access()

If we follow this pattern, then I think that should provide access to either a full coherent set or nothing; and then, the assumption made by the test (36 samples in a cycle, or 0 samples), should be met.

However, the test code (at least the c++ driver) is not following that pattern, and so it is possible to get zero samples from one reader and some samples from another reader within the same 'cycle'.

[sqt]

However, the test code (at least the c++ driver) is not following that pattern, and so it is possible to get zero samples from one reader and some samples from another reader within the same 'cycle'.

As far as I can tell, the C++ harness (and Zig, and hopefully others) do follow this pattern when coherent_set_enabled is true and (set by '-T') and the current write iteration is a multiple of coherent_set_sample_count (set by '--coherent-sample-count'). See here for begin_access() and here for end_access()

The problem this PR is trying to address is that the current set of coherent set tests are testing two things:

1. That the coherent set delivery contract is being honored (no set tearing) and
2. A set of timing/ performance assumptions about how quickly the set can be delivered.

The first constraint is valid for a set of interop tests. The second one is out of scope for spec interoperability, as the spec doesn't define performance criteria. Of particular concern is that the tests are running on cloud VMs which may not strictly start the two processes at exactly the same time (or may suspend one or both depending on load) and the test's read and write cycles may wind up getting fairly out of sync, inadvertently tightening the performance & delivery assumptions further.

So this PR replaces the per-read '36 samples, 3 per instance' assertion with a whole-run check that each instance's sizes are strictly increasing and no coherent set is ever split across a read — which tests the delivery contract without assuming the reader and writer cycles stay in sync.

[ClarkTucker]

The current c++ driver does call begin/end _access(); however it does not call get_datareaders() and limit read/take to the returned set (in this case) of readers. The current approach (call read on each reader in order, every cycle) will allow it to read a 'partial' coherent-set of samples when the timing is 'just right' (or wrong). If we follow this pattern, then I believe that the spec (and a conformant implementation) would deliver either a complete coherent set (36 samples in this case) or nothing. And I believe that is fundamentally all the 'validation' code is looking for. Maybe I'm wrong about that...

[ClarkTucker]

My comments apply only for the 'GROUP' access_scope; so only address tests Test_CoherentSets_12 and Test_CoherentSets_21

[angelrti]

I think that the current test is ok for the GROUP presentation because we have to receive the 36 samples coherent set or 0. There is one edge case that is that you receive 2 groups at the same time, so we can modify the test to receive multiple of 36.

When not using GROUP presentation, the test expects 36 samples, but the coherent set is created per DataWriter. Therefore, we will have coherent sets of 12 samples. So it is not mandatory that we have to receive 36 samples always.

I think that we can reuse the same test and just change the number of samples that we should receive depending on the presentation QOS.

[sqt]

OK, I've simplified the PR to only modify the expected number of samples per polling period, 36 for group presentation and 12 for instance / topic (per-Datawriter). I still think there are underlying timing assumptions at play here which can cause false negatives, but this way the PR's change is much simpler and more in line with @angelrti comment, and in my testing still behaves much better against tests where publisher and subscriber's periods get out of sync.

[sqt]

I've also updated PR title and description.

[angelrti]

I have approved it. Please when merging do the 'squash and merge'. Thanks!