Added macos tests by sougata-progress · Pull Request #10200 · habitat-sh/habitat

sougata-progress · 2026-02-09T22:48:50Z

netlify · 2026-02-09T22:48:58Z

👷 Deploy Preview for chef-habitat processing.

Name	Link
🔨 Latest commit	`9d47213`
🔍 Latest deploy log	https://app.netlify.com/projects/chef-habitat/deploys/69957dcba4f1670008b6db37

Copilot

Pull request overview

Adds macOS unit-test execution to the Expeditor verify pipeline and updates verify scripts to support macOS by using Homebrew instead of Habitat packages.

Changes:

Added macOS unit-test steps for multiple components in the verify pipeline.
Updated shared verify script to skip Habitat license acceptance on macOS.
Updated cargo test runner to perform platform-specific dependency/toolchain setup for macOS.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.

File	Description
.expeditor/verify.pipeline.yml	Adds macOS unit-test steps to the CI verify pipeline.
.expeditor/scripts/verify/shared.sh	Makes license acceptance conditional on OS to support macOS runs.
.expeditor/scripts/verify/run_cargo_test.sh	Adds macOS setup path using Homebrew and adjusts protoc/ZMQ configuration by platform.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Copilot · 2026-02-16T23:02:23Z

 # TODO: fix this upstream, it looks like it's not saving correctly.
-if ${BUILDKITE:-false}; then
+# Only do buildkite-agent chown on Linux systems
+if ${BUILDKITE:-false} && [[ "$OSTYPE" != "darwin"* ]]; then


The negation pattern in the OSTYPE check is incorrect. It should use == instead of != to match the pattern used elsewhere in the file (lines 25, 77), or the logic needs to be inverted.

Suggested change

if ${BUILDKITE:-false} && [[ "$OSTYPE" != "darwin"* ]]; then

if ${BUILDKITE:-false} && [[ "$OSTYPE" == "linux"* ]]; then

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Copilot · 2026-02-17T08:44:57Z

 # This script should contain all shared functions for the verify pipeline

+# Set up writable HAB_ROOT_PATH on macOS to avoid read-only filesystem issues
+if [[ "$OSTYPE" == darwin* ]]; then


The OSTYPE check uses darwin* without quotes around the pattern, while other checks in this PR use "darwin"* with quotes (see run_cargo_test.sh lines 20, 25). For consistency with the rest of the codebase and this PR, this should be "darwin"* with quotes.

Suggested change

if [[ "$OSTYPE" == darwin* ]]; then

if [[ "$OSTYPE" == "darwin"* ]]; then

Copilot · 2026-02-17T08:44:57Z

+  # Create symlinks for the certificate extraction function
+  temp_bin_dir=$(mktemp -d)
+  ln -s "${brew_prefix}/bin/gtail" "${temp_bin_dir}/tail"
+  ln -s "${brew_prefix}/bin/gtar" "${temp_bin_dir}/tar" 


This line has trailing whitespace at the end, which should be removed for consistency with code style.

Suggested change

ln -s "${brew_prefix}/bin/gtar" "${temp_bin_dir}/tar"

ln -s "${brew_prefix}/bin/gtar" "${temp_bin_dir}/tar"

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.

Copilot · 2026-02-17T09:01:44Z

+# Set up writable HAB_ROOT_PATH on macOS BEFORE sourcing shared.sh to avoid read-only filesystem issues
+if [[ "$OSTYPE" == "darwin"* ]]; then
+  export HAB_ROOT_PATH
+  HAB_ROOT_PATH=$(mktemp -d /tmp/hab-root-XXXXXX)
+  # Clean up Darwin-specific temp directory on exit
+  trap 'rm -rf "$HAB_ROOT_PATH"' EXIT
+
+  # Set HAB_LICENSE to skip prompts entirely


The HAB_ROOT_PATH setup code is duplicated between this file and shared.sh (lines 10-14 in shared.sh). This creates a maintenance issue where the same logic exists in two places. Additionally, the code in shared.sh gets sourced after this setup, which may cause the trap cleanup handler to be overridden. Consider removing this block and relying solely on the shared.sh implementation, or move the sourcing of shared.sh to after this block if there's a specific ordering requirement.

Suggested change

# Set up writable HAB_ROOT_PATH on macOS BEFORE sourcing shared.sh to avoid read-only filesystem issues

if [[ "$OSTYPE" == "darwin"* ]]; then

export HAB_ROOT_PATH

HAB_ROOT_PATH=$(mktemp -d /tmp/hab-root-XXXXXX)

# Clean up Darwin-specific temp directory on exit

trap 'rm -rf "$HAB_ROOT_PATH"' EXIT

# Set HAB_LICENSE to skip prompts entirely

# Set HAB_LICENSE on macOS before sourcing shared.sh to skip prompts entirely

if [[ "$OSTYPE" == "darwin"* ]]; then

Copilot · 2026-02-17T09:01:45Z

 set -eou pipefail

+# Set up writable HAB_ROOT_PATH on macOS BEFORE sourcing shared.sh to avoid read-only filesystem issues
+if [[ "$OSTYPE" == "darwin"* ]]; then


The OSTYPE check pattern is inconsistent within this file. Line 6 uses the pattern "darwin"* (with quotes around darwin) while line 10 uses the same pattern darwin* (without quotes on line 10 in shared.sh). While both may work, this inconsistency could lead to confusion. The pattern on line 31 of this file also uses "darwin"*. For consistency, adopt a single pattern throughout, preferably matching what's used elsewhere in the file.

Copilot · 2026-02-17T09:01:45Z

+  export HAB_ROOT_PATH
+  HAB_ROOT_PATH=$(mktemp -d /tmp/hab-root-XXXXXX)
+  # Clean up Darwin-specific temp directory on exit
+  trap 'rm -rf "$HAB_ROOT_PATH"' EXIT


The trap cleanup handler may be overridden when multiple traps are set for the same signal. Since both this file (line 10) and shared.sh (line 14) set a trap for EXIT to clean up HAB_ROOT_PATH, and shared.sh is sourced after this trap is set (line 17), the trap from shared.sh will override this one. This means only one cleanup will occur, which could be problematic if both temporary directories were created. Consider consolidating the trap setup to a single location or using trap additions with the proper trap syntax to chain handlers.

Copilot · 2026-02-17T09:01:45Z

+  temp_bin_dir=$(mktemp -d)
+  ln -s "${brew_prefix}/bin/gtail" "${temp_bin_dir}/tail"
+  ln -s "${brew_prefix}/bin/gtar" "${temp_bin_dir}/tar" 
+  export PATH="${temp_bin_dir}:$PATH"
+
+  macos_use_cert_file_from_linux_cacerts_package
+
+  # Restore original PATH and cleanup
+  export PATH="$original_PATH"
+  rm -rf "$temp_bin_dir"


The temporary directory created at line 72 is cleaned up manually at line 81, but if an error occurs between these lines (e.g., during certificate extraction at line 77), the cleanup won't happen due to the set -eou pipefail at the top of the file. Consider adding this directory to a trap handler to ensure it's cleaned up even on error.

Copilot · 2026-02-17T09:01:46Z

+        macos:
+          os-version: "12"
+          inherit-environment-vars: true
+    timeout_in_minutes: 20


The "[unit] :darwin: core" test step has soft_fail: true (line 627) while the other macOS unit tests (common, hab, http-client) do not. This inconsistency suggests that core tests are expected to fail but should still run. Consider adding a comment explaining why this specific test is allowed to soft fail while others are not, to help future maintainers understand the rationale.

Suggested change

timeout_in_minutes: 20

timeout_in_minutes: 20

# macOS core tests can be flaky / environment-dependent; allow soft failure

# so they still run for signal without failing the entire verify pipeline.

Copilot · 2026-02-17T09:01:46Z

+  # Temporarily modify PATH to prioritize GNU tools over BSD tools
+  brew_prefix=$(brew --prefix)
+  gnu_coreutils_bin="${brew_prefix}/bin"
+  gnu_tar_bin="${brew_prefix}/bin"
+  original_PATH="$PATH"
+  export PATH="${gnu_coreutils_bin}:${gnu_tar_bin}:$PATH"
+
+  # Create symlinks for the certificate extraction function
+  temp_bin_dir=$(mktemp -d)
+  ln -s "${brew_prefix}/bin/gtail" "${temp_bin_dir}/tail"
+  ln -s "${brew_prefix}/bin/gtar" "${temp_bin_dir}/tar" 
+  export PATH="${temp_bin_dir}:$PATH"
+
+  macos_use_cert_file_from_linux_cacerts_package
+
+  # Restore original PATH and cleanup
+  export PATH="$original_PATH"
+  rm -rf "$temp_bin_dir"
+


The approach of creating temporary symlinks and PATH manipulation (lines 72-75) is overly complex and differs from the established pattern. The macos_install_bootstrap_package function already adds /opt/mac-bootstrapper/embedded/bin to the PATH (shared.sh line 388), which provides GNU tail and tar. After calling macos_install_bootstrap_package at line 58, the PATH should already be configured correctly. The symlink creation and PATH juggling is unnecessary and adds complexity. Consider simplifying this by relying on the bootstrap package's PATH setup, which is the pattern used in other scripts like build_mac_hab_binary.sh.

Suggested change

# Temporarily modify PATH to prioritize GNU tools over BSD tools

brew_prefix=$(brew --prefix)

gnu_coreutils_bin="${brew_prefix}/bin"

gnu_tar_bin="${brew_prefix}/bin"

original_PATH="$PATH"

export PATH="${gnu_coreutils_bin}:${gnu_tar_bin}:$PATH"

# Create symlinks for the certificate extraction function

temp_bin_dir=$(mktemp -d)

ln -s "${brew_prefix}/bin/gtail" "${temp_bin_dir}/tail"

ln -s "${brew_prefix}/bin/gtar" "${temp_bin_dir}/tar"

export PATH="${temp_bin_dir}:$PATH"

macos_use_cert_file_from_linux_cacerts_package

# Restore original PATH and cleanup

export PATH="$original_PATH"

rm -rf "$temp_bin_dir"

macos_use_cert_file_from_linux_cacerts_package

Copilot · 2026-02-17T09:01:46Z

+  - label: "[unit] :darwin: common"
+    env:
+      HAB_LICENSE: "accept-no-persist"
+      HOMEBREW_NO_AUTO_UPDATE: 1
+    command:
+      - .expeditor/scripts/verify/run_cargo_test.sh common
+    expeditor:
+      executor:
+        macos:
+          os-version: "12"
+          inherit-environment-vars: true
+    timeout_in_minutes: 20
+    retry:
+      automatic:
+        limit: 1
+
+  - label: "[unit] :darwin: core"
+    env:
+      HAB_LICENSE: "accept-no-persist"
+      HOMEBREW_NO_AUTO_UPDATE: 1
+    command:
+      - .expeditor/scripts/verify/run_cargo_test.sh core
+    expeditor:
+      executor:
+        macos:
+          os-version: "12"
+          inherit-environment-vars: true
+    timeout_in_minutes: 20
+    soft_fail: true
+    retry:
+      automatic:
+        limit: 1
+
+  - label: "[unit] :darwin: hab"
+    env:
+      HAB_LICENSE: "accept-no-persist"
+      HOMEBREW_NO_AUTO_UPDATE: 1
+    command:
+      - .expeditor/scripts/verify/run_cargo_test.sh hab
+    expeditor:
+      executor:
+        macos:
+          os-version: "12"
+          inherit-environment-vars: true
+    timeout_in_minutes: 20
+    retry:
+      automatic:
+        limit: 1
+
+  - label: "[unit] :darwin: http-client"
+    env:
+      HAB_LICENSE: "accept-no-persist"


HAB_LICENSE is set redundantly in the step-level env (lines 601, 617, 634, 650) when it's already defined globally at line 10. This creates unnecessary duplication. Consider removing the step-level HAB_LICENSE environment variable definitions and relying on the global setting, which is the pattern used for other environment variables like HAB_BLDR_CHANNEL.

Copilot · 2026-02-17T09:01:46Z

+        environment:
+          - HAB_LICENSE
+          - HAB_AUTH_TOKEN
+          - HAB_BLDR_CHANNEL


The lint step has been converted from using expeditor.executor.docker to using the docker plugin with explicit configuration. While this change works, the environment variables passed through (lines 63-66) only include HAB_LICENSE, HAB_AUTH_TOKEN, and HAB_BLDR_CHANNEL. The original executor configuration might have automatically passed through other environment variables defined at the global level (lines 11-13), such as HAB_STUDIO_SECRET_HAB_REFRESH_CHANNEL and HAB_REFRESH_CHANNEL. Verify that these environment variables aren't needed for the lint step, or add them to the environment list if they are.

Suggested change

- HAB_BLDR_CHANNEL

- HAB_BLDR_CHANNEL

- HAB_STUDIO_SECRET_HAB_REFRESH_CHANNEL

- HAB_REFRESH_CHANNEL

- HAB_FALLBACK_CHANNEL

- HAB_STUDIO_SECRET_HAB_FALLBACK_CHANNEL

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

Signed-off-by: sougata-progress <sougatab@progress.com>

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Signed-off-by: sougata-progress <sougatab@progress.com>

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

Copilot · 2026-02-17T23:39:24Z

  - label: "[lint] :linux: :paperclip: clippy!"
    command: make lint
-    expeditor:
-      executor:
-        docker:
-          privileged: true
+    agents:
+      queue: 'default-privileged'
+    plugins:
+      docker#v3.3.0:
+        always-pull: true
+        user: "buildkite-agent"
+        group: "buildkite-agent"
+        image: "chefes/buildkite"
+        privileged: true
+        environment:
+          - HAB_LICENSE
+          - HAB_AUTH_TOKEN
+          - HAB_BLDR_CHANNEL
+          - HAB_STUDIO_SECRET_HAB_REFRESH_CHANNEL
+          - HAB_REFRESH_CHANNEL
+          - HAB_FALLBACK_CHANNEL
+          - HAB_STUDIO_SECRET_HAB_FALLBACK_CHANNEL
    timeout_in_minutes: 10


The clippy lint step has been significantly refactored from using the expeditor docker executor to using Buildkite's docker plugin directly with explicit agent queue and environment variable configuration. This change is not mentioned in the PR description, which only states "Added macos tests". Consider documenting this infrastructure change in the PR description to explain why this refactoring was necessary and how it relates to the macOS test additions, or split this change into a separate PR if it's unrelated to the macOS tests.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Signed-off-by: sougata-progress <sougatab@progress.com>

agadgil-progress

I have a very high level comment - I should have given this comment before - but somehow it fell through the cracks. Instead of writing a lot of darwin specific logic inside the run_cargo_test.sh (which also caused some logic in the clippy test to be changed also as a side effect, we could start with a separate run_cargo_test-darwin.sh and then use that script to run the component tests. Do you think it's very difficult to make that change?

Some of the changes that are made to clippy etc are becoming difficult to understand/relate.

sougata-progress · 2026-02-20T19:13:02Z

fixes merged from this pr

Copilot AI review requested due to automatic review settings February 9, 2026 22:48

chef-expeditor bot assigned sougata-progress Feb 9, 2026

Copilot AI reviewed Feb 9, 2026

View reviewed changes

sougata-progress force-pushed the sougata/macos_test branch from 04448a8 to b3e38b1 Compare February 9, 2026 23:33

Copilot AI review requested due to automatic review settings February 10, 2026 07:32

sougata-progress force-pushed the sougata/macos_test branch from b3e38b1 to 01f325f Compare February 10, 2026 07:32

Copilot AI reviewed Feb 10, 2026

View reviewed changes

Comment thread VERSION

Comment thread CHANGELOG.md

Comment thread CHANGELOG.md

Comment thread .expeditor/verify.pipeline.yml Outdated

sougata-progress force-pushed the sougata/macos_test branch 2 times, most recently from f212875 to 2e5f7d0 Compare February 10, 2026 08:52

Copilot AI review requested due to automatic review settings February 10, 2026 08:52

Copilot AI reviewed Feb 10, 2026

View reviewed changes

Comment thread .expeditor/scripts/verify/shared.sh Outdated

Comment thread .expeditor/scripts/verify/run_cargo_test.sh Outdated

Comment thread .expeditor/scripts/verify/run_cargo_test.sh Outdated

Comment thread .expeditor/verify.pipeline.yml Outdated

sougata-progress marked this pull request as ready for review February 10, 2026 10:38

sougata-progress requested a review from a team as a code owner February 10, 2026 10:38

sougata-progress force-pushed the sougata/macos_test branch 2 times, most recently from 8b1ef5f to 60368bf Compare February 10, 2026 11:08

Copilot AI review requested due to automatic review settings February 10, 2026 11:08

Copilot AI reviewed Feb 10, 2026

View reviewed changes

Comment thread .expeditor/scripts/verify/run_cargo_test.sh Outdated

sougata-progress force-pushed the sougata/macos_test branch from 60368bf to 8bd97e6 Compare February 10, 2026 11:17

sougata-progress marked this pull request as draft February 10, 2026 11:18

Copilot AI review requested due to automatic review settings February 10, 2026 11:39

sougata-progress force-pushed the sougata/macos_test branch from 8bd97e6 to 1701ef3 Compare February 10, 2026 11:39

Copilot AI reviewed Feb 10, 2026

View reviewed changes

sougata-progress force-pushed the sougata/macos_test branch from 1701ef3 to 78b156f Compare February 10, 2026 11:49

Copilot AI review requested due to automatic review settings February 11, 2026 12:57

sougata-progress force-pushed the sougata/macos_test branch from 78b156f to 52c85a9 Compare February 11, 2026 12:57

Copilot AI reviewed Feb 11, 2026

View reviewed changes

Comment thread .expeditor/scripts/verify/run_cargo_test.sh Outdated

Comment thread .expeditor/scripts/verify/run_cargo_test.sh Outdated

sougata-progress force-pushed the sougata/macos_test branch from 52c85a9 to 6acdd45 Compare February 11, 2026 13:33

sougata-progress marked this pull request as ready for review February 11, 2026 14:00

Copilot AI review requested due to automatic review settings February 11, 2026 14:00

sougata-progress force-pushed the sougata/macos_test branch from 08780c6 to 78684f6 Compare February 16, 2026 22:34

Copilot AI review requested due to automatic review settings February 16, 2026 22:58

sougata-progress force-pushed the sougata/macos_test branch from 78684f6 to e58758d Compare February 16, 2026 22:58

Copilot started reviewing on behalf of sougata-progress February 16, 2026 22:59 View session

Copilot AI reviewed Feb 16, 2026

View reviewed changes

sougata-progress force-pushed the sougata/macos_test branch from e58758d to acefc6a Compare February 17, 2026 08:33

Copilot AI review requested due to automatic review settings February 17, 2026 08:40

sougata-progress force-pushed the sougata/macos_test branch from acefc6a to a155b33 Compare February 17, 2026 08:40

Copilot started reviewing on behalf of sougata-progress February 17, 2026 08:40 View session