Skip to content

test(lint): assert closed check objects, not an open contract root - #18

Merged
mivds merged 1 commit into
mainfrom
fix/stale-contract-lint-test
Sep 11, 2026
Merged

mivds merged 1 commit into
mainfrom
fix/stale-contract-lint-test

Conversation

@mivds

@mivds mivds commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

Problem

The offline CLI job on main is red. TestContractLint/invalid_contract expects bogus_field at the contract root to fail lint with exit 2, but lint accepts it and exits 0.

This is a semantic merge conflict between two PRs that landed a minute apart:

Each PR was green against its own base, so nothing caught it until both were on main. The lint code is fine; the test's premise went stale.

Change

Points the subtest at an unknown key inside a check rather than at the contract root. Checks are still closed in the synced schema, so this keeps CLI-level coverage of the "additional properties not allowed" error path instead of deleting the case outright.

Alternatives considered

Restoring "additionalProperties": false at the schema root. Rejected: the schema is generated backend-side, so the next sync would revert it, and in the meantime the CLI would reject contracts the backend accepts.

Testing

Both offline tiers — the two jobs CI runs — pass locally:

  • go test ./...
  • go test -tags cli ./tests/integration/... — all 10 TestContractLint subtests pass

Note for reviewers

This class of failure recurs on every backend schema sync. Requiring PRs to be up to date with main before merge (branch protection) would catch it, since both PRs here were individually green.

The offline CLI suite fails on main: TestContractLint/invalid_contract
expects `bogus_field` at the contract root to exit 2, but lint accepts it
and exits 0.

Two PRs merged a minute apart caused this. #16 synced the contract JSON
schema from the backend, which flips the root from
"additionalProperties": false to true — unknown top-level keys are now
allowed by design — and dropped its own TestLintFile_InvalidProperty
accordingly. #17 branched before that landed and added a CLI test
asserting the old behavior. Both were green against their own base; the
merge of the two is not.

The schema is generated backend-side, so pinning the root closed again
would be reverted by the next sync and would reject contracts the
backend accepts. Point the subtest at a check object instead: those are
still closed, so it keeps CLI-level coverage of the "additional
properties not allowed" path rather than deleting it.

Refs: #16, #17

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@mivds mivds added the bug Something isn't working label Sep 10, 2026
@mivds mivds self-assigned this Sep 10, 2026
@mivds
mivds marked this pull request as ready for review September 10, 2026 22:08
@mivds
mivds requested a review from m1n0 September 10, 2026 22:08
@mivds
mivds merged commit 7adcb52 into main Sep 11, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants