Repository navigation
test: make completion test robust to shfmt formatting - #505
Merged
bepri merged 2 commits intoOct 16, 2026
Merged
Conversation
bepri
approved these changes
Oct 16, 2026
dangertoaster
force-pushed
the
work/bot/shfmt-expected-script
branch
from
October 16, 2026 15:00
8f2c4a2 to
6e54cc5
Compare
dangertoaster
added a commit
to dangertoaster/craft-cli
that referenced
this pull request
Oct 16, 2026
Signed-off-by: Callahan Kovacs <callahan.kovacs@canonical.com>
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Normalize both comparison inputs and provide explicit shfmt test coverage or a fallback assertion.
0 open findings
What changed in this PR
Updates the completion integration test and fixture to tolerate shfmt formatting changes.
Changes:
- Reformats the expected Bash completion fixture.
- Normalizes generated output with
shfmt. - Skips the test when
shfmtis unavailable.
Unresolved issues remain: normalize both fixture and generated output, and ensure missing shfmt cannot silently remove test coverage.
| File | Summary |
|---|---|
tests/integration/test_completion/expected_script.sh |
Applies shell formatting to the fixture. |
tests/integration/test_completion.py |
Adds shfmt-based normalization and dependency handling. |
🧠 Review effort: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Ruff I001: craft_cli imports sort before pytest (first-party vs third-party) in this repository's isort configuration.
bepri
marked this pull request as ready for review
October 16, 2026 17:01
This was referenced Oct 16, 2026
lengau
approved these changes
Oct 16, 2026
dangertoaster
added a commit
to dangertoaster/craft-cli
that referenced
this pull request
Oct 20, 2026
Merge `starbase/main` into this branch and sync Starbase-managed updates. Overview of Starbase changes pulled in: - CI workflows: new actionlint, zizmor, and copilot-setup-steps configs; updated check-renovate, policy, release-publish, and TICS workflows - Build tooling: common.mk updates, new lint-code/lint-uv-lockfile/lint-actions targets, and the shared ruff-pre-commit hook configuration - Docs tooling: Sphinx Stack updates (sitemap, llms.txt, sphinx-rerediraffe 0.3.0, meta descriptions on Diátaxis landing pages) - Agent guidance: never invoke the docs Makefile directly (`make -C docs`), use repository-root `make docs-<target>` instead (starbase #636) - osv-scanner configuration Conflict resolution applied: - Kept child-repo version of `.github/CODEOWNERS` (`--ours`). - Kept child-repo version of `.github/workflows/qa.yaml` (`--ours`). - Kept child-repo version of `uv.lock` (`--ours`), then regenerated with `uv lock` for the merged docs dependency group. - Kept child-repo version of `README.md` (Starbase-owned scaffold README is for new projects; this repo's README is already established). - Kept child-repo version of `CONTRIBUTING.md` (library repos keep their repository-specific contribution guide); removed a leftover `|||||||` base marker line from the conflict resolution. - Kept child-repo version of `tests/conftest.py` (`--ours`). - Took full `common.mk` from `starbase/main` (source-of-truth file). - Took full `.github/workflows/check-renovate.yaml`, `.github/workflows/policy.yaml`, and `.github/workflows/release-publish.yaml` from `starbase/main` (source-of-truth files). - `.github/workflows/tics.yaml`: took Starbase structure with the reusable workflow call, keeping `with.project` set to `craft-cli`. - `.pre-commit-config.yaml`: took Starbase hook set, but kept the newer ruff-pre-commit pin from origin/main (v0.17.1 with the renamed `ruff` hook id) per the never-downgrade rule — Starbase is at v0.17.0. - `Makefile`: took Starbase lint/lint-code targets; kept child-owned `PROJECT=craft_cli`; dropped the local `lint-ty`/`install-ty` duplicates now provided by common.mk. - `docs/conf.py`: kept Starbase docs scaffold structure with child-owned values (project, author, copyright start year 2021, ogp_* and html_context URLs, llms_txt_description, LGPL-3.0 license, autodoc/apidoc setup). - `docs/.custom_wordlist.txt`: merged child entries with new shared tooling terms; excluded `Starbase`/`Starcraft` per policy. - `docs/.gitignore`: took Starbase version, appended child repo-specific generated Sphinx file entries. - `docs/**/*.rst`: kept child documentation pages, adopting new meta descriptions and prose from Starbase with `Starcraft` substituted by `Craft CLI`. - `pyproject.toml`: kept child-owned `[project]`, `[dependency-groups]`, `[build-system]`, and ruff src/extend-exclude/lint deltas; took `starbase/main` for `docs-sphinx-stack`, `[tool.setuptools_scm]` structure, and merged `tool.uv.constraint-dependencies` keeping the higher version from each side. Formatted with tombi per the new format-tombi target. - Deleted `docs/release-notes/`, `.github/README.md`, `tests/unit/starbase/test_init.py`, and the leftover `docs/.sphinx/spellingcheck.yaml` (library repository per file-ownership rule 1b; `docs/.sphinx/` superseded by `docs/_dev/`). - For this library repo: kept `AGENTS.md` authoritative and applied relevant updates from `AGENTS.lib.md` (including the new make docs-<target> guidance); kept both `AGENTS.app.md` and `AGENTS.lib.md` deleted. - `tests/unit/test_messages_emitter.py`: removed `# noqa: FBT001`/`FBT002` directives made stale by the merge enabling FBT rules for tests in tool.ruff.lint.per-file-ignores (ruff RUF100). Rebuilt on 2026-10-20 onto current origin/main (which includes the shfmt and tombi preparation PRs canonical#505/canonical#506 and the ruff-pre-commit v0.17.1 bump) and current starbase/main (through 1b92e7b, including the make -C docs guidance from #636).
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 message was generated by kimi-k3.
Problem
tests/integration/test_completion.py::test_completion_outputcompares a generated bash completion script byte-for-byte against the checked-in fixturetests/integration/test_completion/expected_script.sh.The shared Starcraft build tooling (canonical/starbase
common.mk) formats every git-tracked file thatfile --mime-typereports as a shell script usingshfmt -w, explicitly excluding*.sh.j2templates so the Jinja2 source (craft_cli/completion/bash_completion.sh.j2) is not corrupted. However, the expected output fixture is still a plain shell script, so shfmt rewrites it (e.g._complete_testcraft(){→_complete_testcraft() {,all_cmds=( ls cp )→all_cmds=(ls cp)), while the generated output continues to follow the template's original formatting. The byte-for-byte comparison then fails.Fix
shfmtso it matches what repo tooling produces.shfmt -i 2before comparing, so the comparison is formatting-insensitive on both sides. The test skips ifshfmtis not installed.This keeps the test passing regardless of whether the fixture has been reformatted by repo tooling.
Validation
uv run pytest tests/integration/test_completion.py— 2 passeduv run pytest tests/integration -q— 188 passeduv run pytest tests/unit -q— 684 passed, 1 skippedProvenance
Discovered while validating the Starbase sync merge for this repository: the updated
common.mkfrom canonical/starbase addsformat-shfmt/lint-shfmttargets covering all tracked shell scripts. This is a preparation PR for that starbase update.