search

buildkite / test-collector-python

Python adapter for Buildkite Test Analytics
https://buildkite.com/test-analytics
MIT License
6 stars 5 forks source link

fix(pytest_plugin): don't rely on the `pytest_runtestloop` hook to start the payload. #10

Closed jimsynz closed 2 years ago

jimsynz commented 2 years ago

This should fix #3.

matclayton commented 2 years ago

This seems to be an improvement. Thank you!

Its uploading the data correctly, however when swapping to the collector, from uploading junit files, we saw a sudden slowdown in the tests

Screenshot 2022-09-27 at 10 39 37

My observations don't really align with this, they roughly take the same amount of time (72 seconds) as before, so I suspect there is a timing measurement bug somewhere as well.

We saw this on all 6 pipelines, and they all roughly doubled in time. We're running 16x workers, so it is very possible that one is measuring CPU time and one is measuring wall time. Any ideas or suggestions?

jimsynz commented 2 years ago

Hi @matclayton.

Looks like my understanding of xdist's concurrency model was wrong. I have changed the code to start the payload when the first test example starts executing.

Any chance you can try it again for me? Thanks!

matclayton commented 2 years ago

Hi @jimsynz,

No worries, just checked the new build and the timing is still roughly double the original junit ones.

Best, Mat

jimsynz commented 2 years ago

Okay that's confusing. The only thing I can think of is that our use of the monotonic clock is at issue and that perhaps it isn't consistent across xdist workers. I'm not entirely sure how to work around this.

jimsynz commented 2 years ago

A quick follow-up question: how long (wall clock time) does your test run actually take?

matclayton commented 2 years ago

No worries, its about 75s wall clock time, running on 16 cores. Which does imply that the 23mins is more likely correct than the original 13m, however I would surprised the official junit output has this sort of bug in it...Which is what has me stumped.

jimsynz commented 2 years ago

Right. So while I would like to get the timing correct, it seems that being able to visualise relative time changes is pretty useful. I'm going to merge my PR and push a new release as it stands and I'll keep thinking about how to solve this.

matclayton commented 2 years ago

Thanks, I appreciate it and agree with you! I'm not entirely sure that it isn't correct and the bug might be in the old timings.

On Wed, 5 Oct 2022 at 21:21, James Harton @.***> wrote:

Right. So while I would like to get the timing correct, it seems that being able to visualise relative time changes is pretty useful. I'm going to merge my PR and push a new release as it stands and I'll keep thinking about how to solve this.

— Reply to this email directly, view it on GitHub https://github.com/buildkite/test-collector-python/pull/10#issuecomment-1268929580, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAA62CQI3OS3TLL6JUPO5EDWBXPL3ANCNFSM6AAAAAAQWG7FW4 . You are receiving this because you were mentioned.Message ID: @.***>

-- -- Matthew Clayton | Co-Founder Mixcloud Limited

mixcloud http://www.mixcloud.com/mat twitter http://www.twitter.com/matclayton

email @.*** mobile +44 7872007851