Automattic / hostmgr

A tool for managing macOS VM hosts
Mozilla Public License 2.0
8 stars 3 forks source link

Forward `IMAGE_ID` from Buildkite environment #47

Closed mokagio closed 1 year ago

mokagio commented 1 year ago

I wanted to check if the agent had IMAGE_ID =~ ^xcode and run brew install xcodes if so in a pre-commit hook, but it turns out that env var is not available to the scripts.


I tried to add tests for this change but run into a limitation with the setup.

I started by duplicating the test from BuildkiteScriptBuilderTests that verifies that all BUILDKITE_* values are copied and modified it to use IMAGE_ID from a new .env file:

https://github.com/Automattic/hostmgr/blob/40287d22e53a6c593d37a37b3479bd2459eab94e/Tests/libhostmgrTests/BuildkiteScriptBuilderTests.swift#L62-L69

I was surprised to see it passed. I then realized that test verifies the ability of the method to copy all env vars matching the given pattern from the given file. It makes no assertion on whether we do copy BUILDKITE_ env vars at runtime. The component responsible for that behavior is GenerateBuildkiteJobScript from the hostmgr executable target.

I don't know how to test an executable target. As a matter of fact, we do have a draft of test for hostmgr which does not run because not configured in Package.swift, SyncVMImagesCommandTests.

➜ swift test | grep SyncVMImagesCommandTests | wc -l
Compiling plugin GenerateManualPlugin...
Building for debugging...
Build complete! (0.30s)
0

# conversely, with a test that does run
➜ swift test | grep BuildkiteScriptBuilderTests | wc -l
Compiling plugin GenerateManualPlugin...
Building for debugging...
Build complete! (0.31s)
26

I have a "solution" in mind, which I'll propose in a dedicated issue. But for the time being, this change does the job of forwarding IMAGE_ID.

crazytonyli commented 1 year ago

I'm wondering if you need IMAGE_ID for your purpose? Maybe you can check if the pipeline is running on a Mac? Like

[[ $(uname) == 'Darwin' ]] && brew install xcodes
jkmassel commented 1 year ago

I don't know how to test an executable target

So..mostly you can't, which is why I made the libhostmgr package, from which things are easy to test – IMHO ideally the hostmgr tool would just be the CLI bits wrapping the actual functionality in libhostmgr.

WDYT?

mokagio commented 1 year ago

@jkmassel

So..mostly you can't, which is why I made the libhostmgr package, from which things are easy to test – IMHO ideally the hostmgr tool would just be the CLI bits wrapping the actual functionality in libhostmgr.

WDYT

Yeah, that setup seems like a good approach to build a CLI. Functional, testable, fat core; imperative, untested, thin shell. I made a suggestion in #48 about it.

mokagio commented 1 year ago

With xcodes being part of the tools available in the CI images starting 14.1, h/t @crazytonyli , the need for this is purely "just in case, for the future".

Closing.