Refactor CI architecture and enhance documentation for ZUnit#63
Merged
Conversation
Captures the two-tier test architecture decision: native ZUnit on GitHub runners for regular CI, Docker reserved for Zsh version matrix. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Dockerfile now uses ARG ZSH_VERSION via zi pack during build - Matrix workflow: 6 jobs (one per Zsh version) not 30 - entrypoint.sh scope explicitly documented - zi_test variable interpolation behavior noted Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
15-task plan covering zi_test helper, native CI tier, Docker Zsh version matrix, test migration, and cleanup. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…nit in scripts/ - build.sh: Change dockerfile path from "docker/Dockerfile" to "../docker/Dockerfile" (resolves relative to scripts/ after cd) - build.sh: Separate NO_CACHE flag from extra args to prevent silent loss of arguments - run.sh: Add ".." to CONTAINER_ROOT navigation to mount repo root instead of just scripts/ with --dev - run.sh: Initialize DEVEL variable like other boolean flags to prevent environment inheritance
…ck, VOLUME after COPY
…rkflow - Remove docker/tests directory and all ZUnit test files - Remove docker/build.sh, docker/run.sh, docker/zunit.sh, docker/init.zsh - Remove .github/workflows/zunit.yml (testing now handled by CI/CD container) - Update copilot-instructions.md to reflect removed test infrastructure
….yml - Makefile: `make test [FILE=<suite>]` runs ZUnit natively, auto-installs zunit into bin/ on first run; `make run CMD="zi light fzf"` runs an ad-hoc zi command in Docker; `make shell` opens an interactive Docker shell; `make build` wraps scripts/build.sh - test-native.yml: add workflow_call trigger with zi_repo/zi_ref inputs so other repos can test against a specific zi branch/SHA; add same inputs to workflow_dispatch for manual smoke testing; branch install step to use git clone when inputs are provided, install script otherwise
- README.md: project hub with overview, two-tier CI architecture diagram, quick-start commands, image tag table, and navigation links to all docs - docs/local-testing.md: Makefile targets (test/run/shell/build), annotated examples, and full variable reference - docs/ci-workflows.md: two-tier CI model, all workflow triggers and steps, environment variable reference across workflows - docs/cross-repo.md: workflow_call integration guide with end-to-end caller example, input reference, zi_ref strategy, and pinning advice - docs/writing-tests.md: zi_test helper, file/polaris path patterns, variable interpolation rules, test isolation model, adding suites, and common assertion reference
- Add .editorconfig and .geminiignore - Gitignore AI agent instruction files (AGENTS.md, CLAUDE.md, GEMINI.md, .github/copilot-instructions.md) and remove the latter from tracking - Update .trunk/trunk.yaml configuration - Refine CI workflows: codeql.yml, docker.yml, test-native.yml, zsh-n.yml - Update Makefile, README.md, and tests/helpers.zsh - Refresh all docs/: ci-workflows, cross-repo, local-testing, writing-tests - Remove stale AI planning artifacts from docs/superpowers/ - Update docker/Dockerfile, utils.zsh, zshenv, zshrc
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…ntain permissions' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> Signed-off-by: Sall <59910950+ss-o@users.noreply.github.com>
…ntain permissions' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> Signed-off-by: Sall <59910950+ss-o@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR is a substantial restructuring of the zd repo. It moves the test suite from docker/tests/ to a top-level tests/ directory, replaces the run.sh --wrap --debug --zunit test execution model with a new zi_test helper that spawns an isolated Zsh subprocess directly, refactors the Dockerfile into multi-stage form with a new test target, and replaces the existing zunit.yml workflow with two new ones (test-native.yml and test-matrix.yml). Considerable new docs (README.md, docs/local-testing.md, docs/ci-workflows.md, docs/cross-repo.md, docs/writing-tests.md) and a Makefile are added. Several auxiliary GitHub workflows (labeler, lock, pr-labels, rebase, stale, sync-labels) and config files are deleted.
Changes:
- Test infrastructure restructured: tests moved to
tests/, newhelpers.zsh/zi_testhelper,ZI_DATA/ZI_BINenv model replacesDATA_DIR/PLUGINS_DIR/SNIPPETS_DIR/ZPFX. - Dockerfile refactored to a multi-stage build (
base→test), installingzunitandziat build time, dropping the upstream Go install. - CI workflows replaced with
test-native.yml(per-PR, ubuntu-native, alsoworkflow_call-able) andtest-matrix.yml(weekly Zsh-version Docker matrix); many automation workflows removed.
Reviewed changes
Copilot reviewed 44 out of 46 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| docker/Dockerfile | Multi-stage refactor, drops upstream Go, installs zunit/zi at build time |
| docker/entrypoint.sh | Strips user/sudoers/symlink/install setup down to a few sed/mkdir lines |
| docker/zshrc, docker/zshenv, docker/utils.zsh | Reworked to the ZI_BIN/ZI_DATA env model |
| docker/docker-compose.yml | Updated context paths to repo root |
| docker/zunit.sh, docker/init.zsh, docker/tests/* | Deleted; tests moved to repo-root tests/ |
| tests/setup.zsh, tests/teardown.zsh, tests/helpers.zsh | New/updated; introduce zi_test helper and ZI_DATA cleanup |
| tests/{annexes,plugins,packages,snippets,ice}.zunit | Rewritten to use zi_test and ZI_DATA paths |
| scripts/build.sh, scripts/run.sh | Path adjustments for new layout; --zunit/ZUNIT flow removed |
| Makefile | New: test, run, shell, build targets and local zunit install |
| README.md, docs/*.md | New documentation set |
| .github/workflows/test-native.yml, test-matrix.yml | New CI workflows |
| .github/workflows/zunit.yml, labeler.yml, lock.yml, pr-labels.yml, rebase.yml, stale.yml, sync-labels.yml | Deleted |
| .github/workflows/{docker,zsh-n,codeql}.yml | Minor cleanups |
| .github/{copilot-instructions.md,labeler.yml,label-commenter-config.yml} | Deleted |
| .editorconfig | Heavily simplified, drops several language-specific blocks |
| .trunk/trunk.yaml, .trunk/.gitignore, .gitignore | Tool version bumps; gitignore additions |
Comments suppressed due to low confidence (2)
tests/plugins.zunit:19
- The previous fzf test invocation used
as="program" from="gh-r"(Zi ice with=). The new helper-based call drops the=and usesas"program" from"gh-r". While Zi accepts both forms, every other test file in this PR (annexes, packages, snippets) consistently uses the simplezi light <repo>style or no-equals ice form. More importantly, the surrounding test description string literally containsas"program" from"gh-r"— passing this as a single-quoted argument means Zsh inside the inner shell still parsesas"program"correctly, but it is much less readable than the original. Consider either keepingas="program"for consistency with prior code, or using the multi-line'...'form like the other tests in this file.
.github/workflows/zunit.yml:1 - The PR description says ZUnit testing was "improved" by adding
test-matrix.ymlandtest-native.yml, but the existing.github/workflows/zunit.ymlis also being deleted in the same change. The description does not call out the deletion ofzunit.yml, nor does it call out the deletion ofdocker/zunit.sh,docker/init.zsh,docker/tests/setup.zsh, anddocker/tests/ice.zunit(replaced by repo-roottests/). The "Removed outdated documentation" bullet also fails to mention that.github/copilot-instructions.mddescribes a project layout (docker/tests/,zunit.sh,init.zsh) that is being entirely removed by this PR — the file isn't outdated in the abstract; it's being deleted because the PR makes it inaccurate. Consider expanding the description to make the scope of removals/relocations clearer to reviewers.
Dockerfile: - Remove alpine-zsh-config and libuser — not present in Alpine < 3.12; neither is required: busybox adduser handles user creation, zshrc is COPYed from the repo tests/ice.zunit: - Drop assert $state equals 1 from 'failing mv ice' — zi installs the plugin successfully even when mv ice fails (hook failure is non-fatal); assert on the warning message content instead, which is the observable behaviour zi guarantees Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Sall <59910950+ss-o@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Sall <59910950+ss-o@users.noreply.github.com>
Agent-Logs-Url: https://github.com/z-shell/zd/sessions/27d9b13d-f68d-404c-9949-9991426c1034 Co-authored-by: ss-o <59910950+ss-o@users.noreply.github.com>
Agent-Logs-Url: https://github.com/z-shell/zd/sessions/79f9bd39-5098-4b2d-a7af-12304e49bb9c Co-authored-by: ss-o <59910950+ss-o@users.noreply.github.com>
The zi mv hook now reports 'Warning: ∞zi-mv-hook hook returned with 1' instead of the old 'mv ice didn'\''t match any file' message. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Set NO_COLOR=1 and TERM=dumb in the inner zsh so zi does not emit ANSI escape codes. Without this the string assertion assert "$output" contains "∞zi-mv-hook hook returned with" fails because zi wraps the warning in colour codes, splitting the expected substring across escape sequences. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Replace alpine:edge/${ALPINE_VERSION} with debian:trixie-slim (Debian 13,
stable Aug 2025, EOL 2028 / LTS 2030) for glibc compatibility and full apt
package selection
- Dockerfile:
- ARG DEBIAN_FRONTEND → ENV DEBIAN_FRONTEND=noninteractive for reliability
- apk → apt-get install -y --no-install-recommends + rm /var/lib/apt/lists/*
- Package mapping: build-base→build-essential, ncurses-dev→libncurses-dev,
pcre-dev→libpcre2-dev, zlib-dev→zlib1g-dev, go→golang-go
- Added ca-certificates, sudo (not pre-installed on slim images)
- ARG ZSH_VERSION: compile from source when set, else apt install zsh
- WORKDIR reset to / after base stage cleanup
- entrypoint.sh: BusyBox adduser -D → Debian useradd -m; sed /bin/ash →
/bin/bash; fix printf missing trailing newline
- utils.zsh: apk add → apt-get install -y in zi::setup-annexes+add()
- docker-compose.yml: remove deprecated version: '3.9' key
- .github/workflows/docker.yml: ALPINE_VERSION matrix → ZSH_VERSION;
actions/checkout@v6 → @v4
- .github/workflows/test-matrix.yml: setup-buildx-action@v3 → @v4;
build-push-action@v6 → @v7; ZSH_VERSION build-arg now functional
- .github/workflows/zsh-n.yml: fix broken paths: [zi/**] → docker/**,
tests/**; actions/checkout@v6 → @v4
- .gitignore: remove duplicated second half of file (agent-file entries
preserved)
- README.md, docs/: update for Debian base image
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
chmod u+x only sets execute for the file owner (root); the unprivileged container user (uid=1000) could not run these binaries. Use chmod a+x. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Replace wget with curl for zsh tarball download (fixes DL4001: use only one of wget/curl) - Add DL3008 to hadolint ignored list (unpinned apt versions, same rationale as DL3018 already ignored) - Add inline hadolint ignore=DL3003 for conditional cd in source build (WORKDIR cannot be used inside an if/else shell block) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
curl -fsSL fails with exit code 22 (HTTP error) in Docker BuildKit multi-arch CI builds for all versioned zsh targets. Revert to wget which avoids the issue. Add DL4001 to hadolint ignore since wget is needed for zsh tarball download while curl is used elsewhere (revolver/color raw GitHub content). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
zsh.org blocks downloads from GitHub Actions IP ranges (wget exit 8, curl exit 22). Switch to cloning the zsh-users/zsh mirror instead; tags follow the zsh-X.Y.Z format and configure script is present in tagged commits so no tarball or extra tools (wget, xz-utils) are needed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Old zsh versions (5.5.1–5.8.1) declare 'static char *boolcodes[]' in Src/Modules/termcap.c, but Debian trixie's ncurses 6 headers redeclare boolcodes as 'const char * const []'. GCC 14 treats this as a hard 'conflicting types' error. Apply a sed one-liner before ./configure to promote the three bool* arrays to 'const char *const', making them compatible with modern ncurses while preserving correct semantics (they contain string literals). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Old zsh versions (≤5.8.1) define static arrays boolcodes, boolnames, boolfnames, numcodes, numnames, numfnames, strcodes, strnames, strfnames in Src/Modules/termcap.c. Debian trixie's ncurses 6 declares these same names as extern const char * const in term.h. GCC 14 makes this a hard error: static definition cannot follow extern declaration of same name. Rename all 9 arrays to zsh_* prefix via sed before ./configure so there is no symbol collision with the system ncurses headers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
make install fails with exit 2 because nroff/yodl are not installed in the build container, so man page generation fails and 'test -s $file' exits 1 during make install. Use 'make install.bin install.fns install.modules' to install the binary, functions, and modules without documentation — the same fallback used by the zi zsh pack when yodl is absent. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- add automake to apt deps for autoreconf --install --force - replace autoheader+autoconf with autoreconf --install --force which refreshes config.guess/config.sub from system automake data, fixing zsh 5.5.1 build failure on aarch64 (Linux kernel 6.x) - fetch revolver and color.zsh from z-shell/src (SHA-pinned) instead of stale zdharma refs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
Author
|
@copilot apply changes based on the comments in this thread |
Agent-Logs-Url: https://github.com/z-shell/zd/sessions/d95bd222-5b93-4dfd-924a-29030950dc4c Co-authored-by: ss-o <59910950+ss-o@users.noreply.github.com>
Contributor
Fixed in 6ed0a98. Security fix in ZI[HOME_DIR]='${_zi_data}'
source '${_zi_bin}/zi.zsh'Dockerfile COPY path: Already corrected in commit 26ac278. The line now correctly reads |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request makes significant updates to the project's repository and CI/CD configuration, focusing on cleaning up unused files, improving workflow clarity, and enhancing test coverage. The changes remove obsolete or redundant GitHub Actions workflows and configuration files, update the Docker build process for better maintainability, and introduce a new ZUnit test matrix workflow. Additionally,
.editorconfigis streamlined, and documentation/configuration files are removed or updated for clarity.Repository and CI/CD cleanup:
labeler.yml,lock.yml,pr-labels.yml,rebase.yml,stale.yml, andsync-labels.yml, reducing maintenance overhead and potential confusion. [1] [2] [3] [4] [5] [6].github/label-commenter-config.ymlfile, which is no longer needed for automated label commenting..github/labeler.ymlconfiguration, as label automation is no longer used.Continuous integration improvements:
actions/checkoutversions, and improved build context usage. [1] [2] [3] [4]test-matrix.ymlworkflow to run ZUnit tests across a matrix of Zsh versions, improving test coverage and reliability.codeql.ymlworkflow for consistency.Configuration and documentation updates:
.editorconfigby removing lengthy commentary and clarifying indentation and line ending rules for various file types..github/copilot-instructions.mddocumentation file, possibly to reduce duplication or outdated information.