-
Notifications
You must be signed in to change notification settings - Fork 44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow tracing tests to be run in parallel with other tests #95
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, this is absolutely fantastic. Thanks for taking the time to do this.
I've left a few non-blocking nits inline. I'm still going to approve, and leave it to you to decide whether to respond to them, and then run full CI on this.
cdab11e
to
edeaa94
Compare
Full CI, removing If the above jobs look good, I will run a RHEL job too. |
The ci_linux-aarch64 job passed, but the ci_linux job is failing with this weird LTTng error I've never seen when trying to destroy tracing sessions:
It's a direct failure of |
Not off-hand, no. Also, the aarch64 jobs should be exactly equivalent, in the sense that they use the exact same Dockerfile. Let's see what happens on the rebuild, which was launched on a different building host. If we still see it, then we can work with the infrastructure team to try some things out and narrow it down. |
Still failing. I did realize that an ros2_tracing/lttngpy/test/test_session.py Line 57 in ed6f435
|
I re-triggered the aarch64 job and it failed. I'll try to figure out what tests were running in parallel with the failing tests for the Linux jobs, maybe that will give us a clue. |
OK, that sounds good. If you need, one other thing I can do is to run the tests before and after this change back-to-back on the same machine. That would at least tell us whether it is an environmental problem or not. But I'll hold off on doing that until you do some more investigation. |
If my script is correct, these are the packages that are running tests concurrently with
And for
The ci_linux-aarch64 job that passed indeed didn't test |
edeaa94
to
91acfea
Compare
I rebased this after merging #96. Full parallel CI: I don't expect anything different except that |
Signed-off-by: Christophe Bedard <[email protected]>
But make sure the tests do not leave tracing sessions behind. Signed-off-by: Christophe Bedard <[email protected]>
91acfea
to
faf81fe
Compare
So....I made a mistake. Based on a comment from @christophebedard , I was going to add in a change to this PR that made However, by accident I actually pushed that to |
Given that this is much better than before, I'm going to go ahead and merge this one in. I'm sure the improvement is a combination of this PR plus what I accidentally pushed, but that is good enough for me at the moment. |
Resolves #94
This allows tracing tests to be run in parallel with (a) other tracing tests (e.g., from another package) and (b) normal tests (e.g., any ROS 2 tests). It achieves that by doing two things in two separate commits:
test_tracetools
) retrieve that trace test ID from the environment and mark their process by triggering anlttng_ust_tracef:event
event with that trace test ID in its payload, which will link the trace test ID to that process ID, since the trace event includes the PIDlttng_ust_tracef:event
was chosen because it is simple and doesn't require defining a new tracepoint; it simply collects a stringmsg
value, see the lttng_ust_tracef APIlttng_ust_tracef:event
events, collects the PIDs of the events matching its test ID, and filters out trace data from all other processes, leaving only relevant trace datatest_tracetools
andtest_ros2trace
test_ros2trace