[docs] Expand HowToAddABuilder with guidance on testing locally #115024
Conversation
Once <llvm/llvm-zorg#289> and <llvm/llvm-zorg#293> land, it's quite reasonable to ask people to test their builder configurations locally. This patch adds documentation on how to do so. I think review at this stage is useful, but of course if there's more review feedback on <llvm/llvm-zorg#289> it's possible some details may change. This won't be committed until those llvm-zorg PRs land of course.
asb
force-pushed
the
2024q4-document-buildmaster-test-mode
branch
from
6cbd198
to
f31341c
Compare
|
This is beside the point of this change but I wonder if this document could do with a note at the start to the effect that the "master" terminology is what the Buildbot project uses and is therefore used here in that context (see buildbot/buildbot#5382). Since we have precedent for not using the term ourselves, when we moved the master branch to main. |
| Testing a Builder Config Locally | ||
| ================================ | ||
|
|
||
| It's possible to test a builder running against a local version of LLVM's |
There was a problem hiding this comment.
The advice I've been given for this sort of writing is to remove all contractions. ' is fine for possessives but that's about it, pun intended :)
| ================================ | ||
|
|
||
| It's possible to test a builder running against a local version of LLVM's | ||
| buildmaster configuration. This can be helpful to allow quickly identifying |
There was a problem hiding this comment.
"configuration. This allows you to test changes to builder and worker configuration."
Is a bit more succinct.
| buildmaster configuration. This can be helpful to allow quickly identifying | ||
| and iterating over fixes to any issues in either the changes that introduce | ||
| the new builder, or the machine configuration for your worker (preinstalled | ||
| packages etc). A buildmaster launched in this "local testing" mode will bind |
There was a problem hiding this comment.
Since this sentence includes security advice, I would make it into bullet points like:
A buildmaster launced in this "local testing" mode will:
* Bind only to local interfaces
* Use SQLLite as the database
* Use a fixed (single?) password for all workers
* Disable extras like GitHub Authentication
There was a problem hiding this comment.
On the grounds that folks should read and (to themselves) agree with each point in turn.
| pip install buildbot{,-console-view,-grid-view,-waterfall-view,-worker,-www}==3.11.7 urllib3 | ||
|
|
||
| * Initialise the necessary buildmaster files, link to the configuration in | ||
| ``llvm-zorg`` and run a litmus check (run in the directory of your choice): |
There was a problem hiding this comment.
Choice implies there are good and bad choices. This is an arbitrary directory.
How about:
Initialise the necessary buildmaster files, link to the configuration in llvm-zorg and run a litmus check. This step can be run from any directory.
Also I know litmus check is a generic kind of term but maybe just say "and ask buildbot to check the configuration". Then the reader can see the link between that text and the buildbot checkconfig command below.
| ln -s /path/to/checkout/of/llvm-zorg/zorg/ . | ||
| BUILDMASTER_TEST=1 buildbot checkconfig | ||
|
|
||
| * Start the buildmaster using the command below. After a few seconds to |
There was a problem hiding this comment.
"using the command below" is implicit given how the previous steps were written as explanation then command.
I would split "After a few seconds of..." into its own bullet point.
Also note that twistd.log is in the current / same folder.
|
|
||
| BUILDMASTER_TEST=1 buildbot start --nodaemon . | ||
|
|
||
| * With the above in place, you can now create and start a buildbot worker. |
There was a problem hiding this comment.
If I'm following this guide, I know I have the above in place. If you want to emphasize that they need to deal with any errors first you could say this instead:
Assuming there were no errors from the previous step, ...
| * With the above in place, you can now create and start a buildbot worker. | ||
| Ensure you pick the correct name for the worker attached to the build | ||
| configuration you want to test in | ||
| ``buildbot/osuosl/master/config/builders.py``. After doing the below, either |
There was a problem hiding this comment.
Instead of making the reader refer back to this part, just move this below the command. Then the text is in order and there's no need for "after the below".
There was a problem hiding this comment.
Remember that bullet points can have multiple paragraphs if you keep the indentation the same.
| buildbot-worker start --nodaemon <buildbot-worker-root-directory> | ||
|
|
||
| This local testing configuration defaults to binding only to the loopback | ||
| interface for security reasons. If you want to run the test worker on a |
There was a problem hiding this comment.
I would add a blank line here for a bit of emphasis that the default is the default for a reason.
| the web UI accessible via ``http://localhost:8011`` and make it possible for a | ||
| local worker to connect to the remote buildmaster by connecting to | ||
| ``localhost:9900``: ``ssh -N -L 8011:localhost:8011 -L 9990:localhost:9990 | ||
| username@server_address``. |
There was a problem hiding this comment.
.. code block for this?
Once llvm/llvm-zorg#289 and llvm/llvm-zorg#293 land, it's quite reasonable to ask people to test their builder configurations locally. This patch adds documentation on how to do so.
I think review at this stage is useful, but of course if there's more review feedback on llvm/llvm-zorg#289 it's possible some details may change. This won't be committed until those llvm-zorg PRs land of course.