launch testing : Wait for topics to publish#274
Conversation
Signed-off-by: Aditya Pande <aditya050995@gmail.com>
Signed-off-by: Aditya Pande <aditya050995@gmail.com>
Signed-off-by: Aditya Pande <aditya050995@gmail.com>
Signed-off-by: Aditya Pande <aditya050995@gmail.com>
jacobperron
left a comment
There was a problem hiding this comment.
I think this is a good start. I've left some feedback below.
Also, I'm not sure that creating a separate node class adds much value; I would have just put all the logic inside WaitForTopics. I'm okay with it either way though.
| self.create_subscription( | ||
| topic_type, | ||
| topic_name, | ||
| self.callback_template(topic_name), |
There was a problem hiding this comment.
Alternatively, we can use functools.partial.
functools.partial(self.topic_callback, topic_name)and then the callback would look like:
def topic_callback(self, topic_name, data):
...I don't think there's anything wrong with this, so feel free to leave it as-is.
Signed-off-by: Aditya Pande <aditya050995@gmail.com>
Signed-off-by: Aditya Pande <aditya050995@gmail.com>
jacobperron
left a comment
There was a problem hiding this comment.
LGTM, two more comments below
Signed-off-by: Aditya Pande <aditya050995@gmail.com>
Signed-off-by: Aditya Pande <aditya050995@gmail.com>
Signed-off-by: Aditya Pande <aditya050995@gmail.com>
Signed-off-by: Aditya Pande <aditya050995@gmail.com>
jacobperron
left a comment
There was a problem hiding this comment.
A couple suggestions for the test code. LGTM
Signed-off-by: Aditya Pande <aditya050995@gmail.com>
|
👨🌾 It seems that this PR caused several regressions in the WIndows buildfarms jobs: nightly_win_rel#2079 Do you think you can take a look to a possible fix @adityapande-1995? I think it's likely related to these lines: It seems to me that |
|
It's weird that this caused a regression, particularly because CI seems to have been run properly and came back green. But let's see if the revert PR fixes it. |
This reverts commit bbcc0cc. Signed-off-by: Jorge Perez <jjperez@ekumenlabs.com>
|
I can't replicate the error in CI. I'll investigate a bit deeper before reaching any conclusion. |
I believe that's for this. Not sure why it can't be imported before running tests though. |
This aims to close #272. It aims to provide an API in to be used with
launch_testingwhere we provide a set of topics and we wait for at least a single message to appear on all of them.Sample usage :