Skip to content

Conversation

@asb
Copy link
Contributor

@asb asb commented Nov 10, 2024

This provides a stronger check that the buildbot configuration is correct than just running our minimal lit tests.


Stacks on top of the following:

I've renamed the workflow from "lit-tests.yml" to "litmus-tests.yml". The required set of installed dependencies and steps are essentially the same and running the tests and checkconfig takes only a few seconds, so it would seem wasteful to split into independent parallel workflows rather than just running them sequentially.

See here for an example of the action failing (from a run from before I'd cherry-picked #293) https://github.com/asb/llvm-zorg/actions/runs/11766043316/job/32773083955 and of course the run from this PR succeeding in this repo is https://github.com/llvm/llvm-zorg/actions/runs/11766114836/job/32773247437?pr=303

asb added 4 commits November 10, 2024 13:58
Installing a specific version of buildbot is more robust to future
changes vs using whatever is packaged for Debian.
BUILDBOT_TEST is sufficiently descriptive (after all, 'buildbot' is the
package name and the name of the binary invoked to create a buildmaster)
and it's possible upstream may later remove the buildmaster terminology
<buildbot/buildbot#5382> or alternatively we
might move away from it downstream. As such, it seems prudent to use a
name that would be unaffected.
…ength

Upstream buildbot releases error if a step name has over 50 characters
long <buildbot/buildbot#3414>.
UnifiedTreeBuilder can produce quite long step names that violate this,
meaning it's difficult to spin up a local test environment (as in
<llvm#289>) without local hacks to
workaround this.

In this patch, we simply truncate step names at creation time. This
means llvm-zorg's buildbot config can be used with an unmodified
upstream buildbot package.
This provides a stronger check that the buildbot configuration is
correct than just running our minimal lit tests.
Copy link
Contributor

@DavidSpickett DavidSpickett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@asb asb merged commit 0d9778f into llvm:main Nov 13, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants