Skip to content

test: fix shfmt error message assertions - #1393

Merged
skovhus merged 1 commit into
bash-lsp:mainfrom
kolyshkin:shfmt-error-message-quoting
Aug 19, 2026
Merged

test: fix shfmt error message assertions#1393
skovhus merged 1 commit into
bash-lsp:mainfrom
kolyshkin:shfmt-error-message-quoting

Conversation

@kolyshkin

Copy link
Copy Markdown
Contributor

The formatter tests fail against shfmt >= 3.13 (I hit this on Fedora with shfmt 3.13.1, on current main, unrelated to any local change).

Changed error messages

mvdan/sh 16cc925dc ("syntax: consistently quote tokens in error messages") landed in shfmt 3.13.0, so two messages the tests match on now look different:

-  parse-problems.sh:10:1: > must be followed by a word
+  parse-problems.sh:10:1: `>` must be followed by a word

-  shfmt.sh:25:14: the "function" builtin is a bash feature; tried parsing as posix
+  shfmt.sh:25:14: the `function` builtin is a bash feature; tried parsing as posix

The assertions now accept either quoting style, alongside the pre-existing alternative for much older shfmt versions.

CI does not see this because verify.yml installs shfmt via apt-get, and the version in the Ubuntu archive predates 3.13.

Missing await

While tracking this down: the three expect(...).rejects.toThrow(...) assertions in this file were never awaited or returned. The test therefore finished green and the rejection surfaced during whichever test ran next — so jest blamed should format when shfmt is present for a message belonging to should throw when parsing using the wrong language dialect, which made this a good deal more confusing than it needed to be.

Verified the assertions actually bite now: deliberately corrupting the expected strings fails the tests that own them, rather than their neighbours.

shfmt 3.13.0 started quoting tokens in error messages consistently
(mvdan/sh 16cc925dc), so the messages the formatter tests match on
changed:

    parse-problems.sh:10:1: > must be followed by a word
    parse-problems.sh:10:1: `>` must be followed by a word

    shfmt.sh:25:14: the "function" builtin is a bash feature
    shfmt.sh:25:14: the `function` builtin is a bash feature

Accept either form. CI does not see this yet, as the shfmt in the
Ubuntu archive is older than 3.13.

These failures were also being reported against the wrong test: the
three `expect(...).rejects.toThrow(...)` assertions were never awaited,
so the test finished green and the rejection surfaced during whichever
test happened to run next. Await them.
@kolyshkin

Copy link
Copy Markdown
Contributor Author

Opened #1394 which is this PR plus the CI addition to also test against the latest shfmt.

@skovhus
skovhus self-requested a review August 19, 2026 07:48
@skovhus
skovhus enabled auto-merge August 19, 2026 07:48
@skovhus
skovhus merged commit 77294ff into bash-lsp:main Aug 19, 2026
4 checks passed
@codecov

codecov Bot commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.56%. Comparing base (e46e4a6) to head (d14b5d2).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1393   +/-   ##
=======================================
  Coverage   80.56%   80.56%           
=======================================
  Files          29       29           
  Lines        1513     1513           
  Branches      374      374           
=======================================
  Hits         1219     1219           
  Misses        235      235           
  Partials       59       59           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants