Skip to content

fix(ci): repair failing lint and test jobs#14

Merged
hahwul merged 1 commit into
mainfrom
fix/ci-lint-and-tests
May 30, 2026
Merged

fix(ci): repair failing lint and test jobs#14
hahwul merged 1 commit into
mainfrom
fix/ci-lint-and-tests

Conversation

@hahwul

@hahwul hahwul commented May 30, 2026

Copy link
Copy Markdown
Owner

Why

CI on main has been failing on both the lint and tests jobs since the refactor in 00b1ee8.

Root cause

  • tests (ubuntu + macos): spec/spec_helper.cr required ../lib/webmock/src/webmock, described as "vendored... committed in tree". But lib/ is gitignored, webmock was never committed, and it wasn't a declared dependency — so CI's clean shards install only fetched ameba and spec compilation failed with can't find file '../lib/webmock/...'.
  • lint: the same commit enforced ameba by removing continue-on-error: true, but never cleaned up the ~184 pre-existing violations and added no config.

What changed

Tests

  • Add webmock as a proper development_dependency (shard.yml / shard.lock).
  • Require it normally in spec_helper.cr; drop the misleading comment and the unused require_webmock! helper.

Lint

  • Add .ameba.yml that disables only rules conflicting with deliberate project conventions, each with a rationale:
    • Naming/VariableNames — camelCase vars (createTime, gitPatch, …) mirror the Jules REST API JSON fields 1:1.
    • Naming/BlockParameterName — short block params used idiomatically.
    • Metrics/CyclomaticComplexity — CLI dispatcher / flag parsers are inherently branch-heavy.
    • Lint/NotNil — flag values are captured in OptionParser closures, where Crystal flow typing can't narrow and not_nil! is the idiomatic guard (plus spec fixtures).
  • Fix the remaining real issues in code (not config): RedundantBegin, VerboseBlock, negated unless, lines.eacheach_line, .sort.sort!, the x.not_nil!.foo if x pattern → if x = ..., and getter quit_earlygetter? quit_early?.

Robustness (prevents recurrence)

  • Pin the CI lint job to the ameba version locked in shard.lock (bin/ameba) instead of crystal-ameba/github-action@master. The @master action always runs bleeding-edge ameba, so a new upstream rule (e.g. the Signal::INT.trap one that fires on master but not local 1.6.4) could break CI with no code change. Now local just lint == CI.

Verification (local)

shards build                  → OK
bin/ameba                     → 44 inspected, 0 failures
crystal spec                  → 97 examples, 0 failures
crystal tool format --check   → OK

The previous refactor (00b1ee8) broke CI in two ways:

- tests: spec_helper required a "vendored" webmock under lib/ that is
  gitignored and never committed, and was not a declared dependency, so
  CI's clean `shards install` could not find it. Add webmock as a proper
  development_dependency and require it normally.

- lint: the refactor enforced ameba (removed continue-on-error) without
  cleaning up existing violations or adding config. Add a principled
  .ameba.yml that disables only rules conflicting with deliberate project
  conventions (camelCase vars mirroring the Jules API JSON, short block
  params, CLI dispatcher complexity, not_nil! on closure-captured flags),
  and fix the remaining real issues in code (RedundantBegin, VerboseBlock,
  negated unless, lines.each -> each_line, sort -> sort!, the
  x.not_nil!.foo if x pattern, and getter -> getter? for quit_early?).

Also pin the CI lint job to the ameba version locked in shard.lock
(bin/ameba) instead of @master, so local `just lint` and CI agree and an
upstream ameba rule change can no longer break CI without a code change.

Verified locally: shards build, bin/ameba (0 failures), crystal spec
(97 examples, 0 failures), crystal tool format --check all pass.
@hahwul hahwul merged commit deafeaf into main May 30, 2026
8 checks passed
@hahwul hahwul deleted the fix/ci-lint-and-tests branch May 30, 2026 07:40
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.

1 participant