have_one_new_sample raises timeout failure when executed in a loop

[gustavoneves12]

The method have_one_new_sample is raising a timeout failure when expect_execution is executed in a loop as shown in the code below:

samples.each |sample| do
    actual = expect_execution { syskit_write task.in_port, sample}
             .to { have_one_new_sample task.out_port }
end

It seems some samples have been lost. In my tests cases, samples have 120 elements. To make it works, I created a reader out of the expect_execution.to block as in the code below:

reader = syskit_create_reader task.out_port
samples.each |sample| do
    actual = expect_execution { syskit_write task.in_port, sample}
             .to { have_one_new_sample reader }
end

Did I do something wrong?
Why the first code did not work and the second one works?

[doudou]

It depends on how your task behaves.

expect_execution ... to ... is self contained. This means that it creates a reader each single time. It does so just before the syskit_write and then closes it afterwards.

If you really mean to test that there is a 1:1 match between a in sample and an out sample, the test you originally wrote is really what you want (and I'd ask: why do you think it's a syskit bug ?)

If you want to test that your task receives a certain number of samples after having written a certain number of samples (but don't care about the 1:1 mapping), there is no available predicate right now. So, I'd go for

reader = syskit_create_reader task.out_port, type: :buffer, size: 10
syskit_write task.in_port, *samples
(0...10).map { expect_execution.to { have_one_new_sample reader } }

Obviously, all of it would be better factored out in a new have_new_samples predicate. That exercise is left to the reader...

[gustavoneves12]

If you really mean to test that there is a 1:1 match between a in sample and an out sample, the test you originally wrote is really what you want (and I'd ask: why do you think it's a syskit bug ?)

I would like to test a 1:1 matching. My task is port_driven and it should produce an output for each new input. However, it just works when I create the reader out of expect_execution ... to .... It is important to note that it does not break in the first cycle of the loop, it processes about 80 to 90 element before raising the timeout failed. The number of processed elements is reduced when I include a sleep inside samples.each after the expect_execution ... to ....

I am not sure if this is a syskit bug. However, for testing this, I created a simple orogen task that receives an Angle and outputs the received Angle, not doing any process inside the updateHook. The update hook code is shown below:

base::Angle angle;
while (_in_samples.read(angle) == RTT::NewData) {
    _out_samples.write(angle);
}

[doudou]

Mmmm ... I do agree with your assessment, it does seem to be a Syskit or Syskit/RTT bug.

One thing you could try is to display "CONNECTED" / "NOT CONNECTED" just before the write in the component. If there's a race in the expect_execution harness, the port should not be connected and we should see it there.

[gustavoneves12]

What you suggested is something like that?

base::Angle angle;
while (_in_samples.read(angle) == RTT::NewData) {
    if (_out_samples.connected())
        std::cout << "CONNECTED" << std::endl;
    else
        std::cout << "NOT CONNECTED" << std::endl;
    _out_samples.write(angle);
}

I have done this test.

The in_expect_execution.txt is the output with the reader inside the expect_execution.

The out_expect_execution.txt is with the reader out of the expect_execution.

With the reader inside the expect_execution we got a lot the message below:

0.503 [ ERROR  ][OrbRunner] caught CORBA exception while reading a remote channel: OBJECT_NOT_EXIST

[doudou]

Hey. I'm sorry, but I won't have a lot more time to look at it today.

The error message seems to indicate that the component reads its input port while the writer is being disconnected. This is not totally unexpected - the second call to read in the while could happen during the disconnection. But then I don't see why there should be more of it in the in case than the out case.

[doudou]

Ok. So, new insight from #247

The test code is not disconnecting the readers after the expect_execution block. This means that each iteration is creating a new reader but at the same time keeps the old one.

It turns out that there is super-linear complexity in RTT w.r.t. the number of channels attached to a port. My guess is that reading is getting simply too slow.

The obvious bugfix would be to ensure that the readers/writers created during the tests are disconnected at the end of the test.

[gustavoneves12]

For you information, after this PR #246, I have changed the tests as shown below. It worked as expected.

 actuals = expect_execution { syskit_write task.in_port, *samples }
                  .to { have_new_samples task.out_port, size }
expected.each_with_index do |exp, i|
    assert_equal exp, actuals[i]
end

[doudou]

It worked as expected.

Good workaround, but then you're not testing exactly what the component is supposed to be doing, that is writing one sample for each sample read (because of the Syskit bug). You could for instance write more than one and still have size samples.

[doudou]

@gustavoneves12 why the label change. You have not asked anything since my last reply ?

In addition, the labels are for the developers to triage the issues. A.k.a. "help wanted" and "question" is for the developers to say "we need people to answer our questions before we can go on". It's not for the person asking to say "it's urgent to me". Because, it's always urgent to the person that's asking

[gustavoneves12]

@doudou I just changed the labels for convenience. Because I would like to classify this issue as a question since at that time when I opened this issue, I didn't know if this is an issue or not. In my point of view issue always sound like a bug. Sorry, I didn't want to say it is urgent to me. Should I remove these labels?

[doudou]

Labels are not for the convenience of the person that report them, but for the developers. They're here to triage the issues for development.

So, unless you are willing to try and fix issues (please do!), they are not for you to use.