buildmaster-config icon indicating copy to clipboard operation
buildmaster-config copied to clipboard

Add "test-with-buildbots" label on a Python PR runs also Refleaks buildbot

Open vstinner opened this issue 2 years ago • 5 comments

Example of PR: https://github.com/python/cpython/pull/110458

Multiple Refleaks buildbots were run:

  • AMD64 RHEL7 Refleaks PR
  • s390x Fedora Refleaks PR
  • AMD64 RHEL8 Refleaks PR
  • AMD64 Windows11 Refleaks PR
  • etc.

vstinner avatar Oct 06 '23 10:10 vstinner

I thought that was deliberate. We've had the test-with-buildbots label for a long time, and it always ran all buildbots. The test-with-refleak-buildbots label is a new innovation to just run a subset of buildbots. As I understood it, I don't think the plan was to ever change the existing label.

AlexWaygood avatar Oct 06 '23 10:10 AlexWaygood

Currently, we have:

  • "test-with-buildbots" tests all buildbots including Refleaks (slow)
  • "test-with-refleak-buildbots" only tests Refleaks buildbots (slow)

I would prefer that:

  • "test-with-buildbots" tests all buildbots excluding Refleaks (quick)
  • "test-with-refleak-buildbots" tests *all buildbots including Refleaks (slow)

If people are used to the current behavior, maybe what I need is a 3rd label:

  • "test-with-buildbots" tests all buildbots including Refleaks (slow)
  • "test-without-refleak-buildbots" tests all buildbots excluding Refleaks (quick)
  • "test-with-refleak-buildbots" only tests Refleaks buildbots (slow)

But it might be misleading.

vstinner avatar Oct 06 '23 10:10 vstinner

Refleaks builders are very slow, they should not be abused. A single build can take 2 to 4 hours.

vstinner avatar Oct 06 '23 10:10 vstinner

I've seen lots of people add both labels to PRs, so I think there are definitely people who find the current behaviour confusing. But I think there are also lots of people who are used to the current behaviour. So if this changes, you'll need to make sure you communicate the change loudly on Discourse and Discord :-)

AlexWaygood avatar Oct 06 '23 10:10 AlexWaygood

I agree with Victor on pretty much every point here.

If we want to keep it to two labels, I'd rather rename test-with-buildbots to clarify that it doesn't run refleak builds, and not run refleak builds with it.

zware avatar Oct 06 '23 17:10 zware

No activity for almost one year, I close the issue. While the issue is annoying, it's not a major burden :-) If someone wants to propose a fix, you can reopen the issue.

vstinner avatar Sep 06 '24 14:09 vstinner