diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index a5b02daec..3735a0a23 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -45,6 +45,33 @@ jobs: - name: Verify embedded provenance is up to date run: diff -q spec/api-provenance.json go/pkg/basecamp/api-provenance.json + fixture-coverage: + name: Fixture coverage + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + persist-credentials: false + + - name: Set up Ruby + uses: ruby/setup-ruby@d45b1a4e94b71acab930e56e79c6aa188764e7f9 # v1.316.0 + with: + ruby-version: "3.3" + + # Run under LC_ALL=C so CI actually exercises the non-UTF-8-locale path + # (the reads are pinned to UTF-8; this proves it stays that way). + - name: Check fixture completeness + coverage invariant + run: ruby scripts/check-fixture-coverage.rb + env: + LC_ALL: C + + - name: Fixture-guard self-test (negative + synthetic cases) + run: ruby scripts/test-check-fixture-coverage.rb + env: + LC_ALL: C + test-go: name: Go Tests runs-on: ubuntu-latest diff --git a/AGENTS.md b/AGENTS.md index a23b4cf81..197a4d2bb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -309,9 +309,22 @@ Use `make sync-status` to see upstream diffs since last sync. ### Every Changed Field/Path Requires -1. **Existing tests updated** — every test stubbing a changed path must be updated +1. **Existing tests updated** — every test stubbing a changed path must be updated. + Inline stubs may omit unrelated response fields, but every response stub for a + changed path must include any newly required or behaviorally changed field. 2. **New field tests** — at least one test fixture should include new fields to verify they flow through +> **Automated backstop.** The shared JSON fixtures under `spec/fixtures/` are +> guarded by `make check-fixture-coverage` (`spec/fixtures/manifest.yaml`): every +> manifest'd fixture is validated against its schema for both required-field +> presence and type/nullability (a required-null-against-non-nullable or a +> wrong-typed value fails), every `covered_schemas` entry must keep a concrete +> representative, and every rich-text emitter schema must be covered or explicitly +> excluded (so the inventory can't silently shrink). A new required field on a +> covered schema is therefore forced into a fixture. This guard covers only the +> manifest'd shared fixtures — one-off inline stubs remain the reviewer's +> responsibility under the rule above. + ### Pre-Merge Verification Run `make go-check-drift`, `make kt-check-drift`, and `make py-check-drift` (all included in `make check`) and verify: diff --git a/Makefile b/Makefile index 9d215b884..a278d8988 100644 --- a/Makefile +++ b/Makefile @@ -743,7 +743,7 @@ tools: # Spec-shape lints #------------------------------------------------------------------------------ -.PHONY: check-bucket-flat-parity validate-api-gaps check-deprecation-parity kt-check-optional-arrays +.PHONY: check-bucket-flat-parity validate-api-gaps check-deprecation-parity check-fixture-coverage kt-check-optional-arrays # Verify every bucket-scoped GET list operation has a flat-path counterpart # (or is justified in spec/bucket-scoped-allowlist.txt). Cross-project SDK @@ -765,6 +765,16 @@ check-deprecation-parity: rb-build validate-api-gaps: @./scripts/validate-api-gaps.sh +# Fixture-completeness guard: every spec/fixtures/manifest.yaml target validates +# against its schema (required-field presence + type/nullability), every covered +# schema keeps a concrete active representative, and every rich-text emitter is +# accounted for — so a new required field on a covered schema is forced into a +# fixture. Reuses the conformance schema-walker. The self-test asserts the guard +# rejects each crafted failure mode (the live check only exercises the valid set). +check-fixture-coverage: + @./scripts/check-fixture-coverage.sh + @ruby ./scripts/test-check-fixture-coverage.rb + # D-invariant: every optional generated Kotlin array is `List? = null`, every # required array stays `List`, and none default to the `= emptyList()` # sentinel — pinning the optional-array presence contract against regression. @@ -793,7 +803,7 @@ generate: @echo "==> Generation complete" # Run all checks (Smithy + Go + TypeScript + Ruby + Kotlin + Swift + Python + Behavior Model + Conformance + Provenance + Actions lint) -check: lint-actions sync-spec-version-check smithy-check behavior-model-check provenance-check sync-api-version-check go-check-drift go-check-wrapper-drift auth-routable-check kt-check-drift go-check ts-check rb-check kt-check swift-check py-check conformance check-bucket-flat-parity validate-api-gaps check-deprecation-parity kt-check-optional-arrays +check: lint-actions sync-spec-version-check smithy-check behavior-model-check provenance-check sync-api-version-check go-check-drift go-check-wrapper-drift auth-routable-check kt-check-drift go-check ts-check rb-check kt-check swift-check py-check conformance check-bucket-flat-parity validate-api-gaps check-deprecation-parity check-fixture-coverage kt-check-optional-arrays @echo "==> All checks passed" # Clean all build artifacts @@ -906,6 +916,6 @@ help: @echo "" @echo "Combined:" @echo " generate Regenerate every machine-derived artifact (Smithy + per-language SDKs + provenance)" - @echo " check Run all checks (Smithy + behavior-model/drift + Go + TypeScript + Ruby + Swift + Kotlin + Python + Conformance + Provenance + API version sync + parity lint + api-gaps + Actions lint)" + @echo " check Run all checks (Smithy + behavior-model/drift + Go + TypeScript + Ruby + Swift + Kotlin + Python + Conformance + Provenance + API version sync + parity lint + api-gaps + fixture-coverage + kt-optional-arrays + Actions lint)" @echo " clean Remove all build artifacts" @echo " help Show this help" diff --git a/conformance/runner/ruby/schema-walker.rb b/conformance/runner/ruby/schema-walker.rb index 843556e8f..02d018d8d 100644 --- a/conformance/runner/ruby/schema-walker.rb +++ b/conformance/runner/ruby/schema-walker.rb @@ -30,7 +30,9 @@ class SchemaWalker MAX_DEPTH = 12 def initialize(openapi_path) - @doc = JSON.parse(File.read(openapi_path)) + # Read as UTF-8 regardless of process locale (LC_ALL=C would otherwise + # read as US-ASCII and choke on the spec's UTF-8 bytes). + @doc = JSON.parse(File.read(openapi_path, encoding: "UTF-8")) end # Returns the response schema for operation_id, or nil when none exists. diff --git a/python/tests/services/test_todos.py b/python/tests/services/test_todos.py index 1531b7565..af9fe9550 100644 --- a/python/tests/services/test_todos.py +++ b/python/tests/services/test_todos.py @@ -3,6 +3,7 @@ from __future__ import annotations import json +from pathlib import Path import httpx import pytest @@ -14,9 +15,19 @@ BASE = "https://3.basecampapi.com/12345" +_FIXTURES = Path(__file__).resolve().parents[3] / "spec" / "fixtures" + + +def load_fixture(rel: str) -> dict: + return json.loads((_FIXTURES / rel).read_text(encoding="utf-8")) + def _todo(todo_id: int = 42, **overrides) -> dict: - todo = { + # Source the full validated fixture for shape (every required Todo field is + # present), then keep the test-critical override values that the assertions + # verify flow through to the PUT body. + return { + **load_fixture("todos/get.json"), "id": todo_id, "content": "Buy milk", "description": "

From the store

", @@ -25,9 +36,8 @@ def _todo(todo_id: int = 42, **overrides) -> dict: "assignees": [{"id": 100, "name": "Jane Doe"}], "completion_subscribers": [{"id": 555, "name": "Sub Scriber"}], "completed": False, + **overrides, } - todo.update(overrides) - return todo def _put_body(route) -> dict: diff --git a/python/tests/services/test_uploads.py b/python/tests/services/test_uploads.py index a008526b5..1af3c9f28 100644 --- a/python/tests/services/test_uploads.py +++ b/python/tests/services/test_uploads.py @@ -2,6 +2,9 @@ from __future__ import annotations +import json +from pathlib import Path + import httpx import pytest import respx @@ -9,14 +12,20 @@ from basecamp import AsyncClient, Client from basecamp.errors import UsageError +_FIXTURES = Path(__file__).resolve().parents[3] / "spec" / "fixtures" + + +def load_fixture(rel: str) -> dict: + return json.loads((_FIXTURES / rel).read_text(encoding="utf-8")) + def _metadata(upload_id: int = 1069479400, *, download_url, filename="report.pdf") -> dict: - """Minimal upload metadata payload; callers override download_url/filename.""" + """Upload metadata sourced from the validated fixture; callers override download_url/filename.""" return { + **load_fixture("uploads/get.json"), "id": upload_id, "filename": filename, "download_url": download_url, - "description_attachments": [], } diff --git a/ruby/test/basecamp/services/cards_service_test.rb b/ruby/test/basecamp/services/cards_service_test.rb index e25d75a7f..7d669371b 100644 --- a/ruby/test/basecamp/services/cards_service_test.rb +++ b/ruby/test/basecamp/services/cards_service_test.rb @@ -14,16 +14,9 @@ def setup @account = create_account_client(account_id: "12345") end - def sample_card(id: 1, title: "Task Card") - { - "id" => id, - "title" => title, - "content" => "

Card description

", - "description_attachments" => [], - "due_on" => "2024-12-31", - "completed" => false, - "assignees" => [] - } + def sample_card(id: nil, title: nil) + fixture = load_fixture("cards/get.json") + fixture.merge "id" => id || fixture["id"], "title" => title || fixture["title"] end def test_list_cards @@ -33,7 +26,7 @@ def test_list_cards cards = @account.cards.list(column_id: 200).to_a assert_equal 2, cards.length - assert_equal "Task Card", cards[0]["title"] + assert_equal "Implement user authentication", cards[0]["title"] assert_equal "Another Card", cards[1]["title"] end @@ -44,7 +37,7 @@ def test_get_card card = @account.cards.get(card_id: 200) assert_equal 200, card["id"] - assert_equal "Task Card", card["title"] + assert_equal "Implement user authentication", card["title"] end def test_create_card diff --git a/ruby/test/basecamp/services/checkins_service_test.rb b/ruby/test/basecamp/services/checkins_service_test.rb index 0a9018c01..b74db6298 100644 --- a/ruby/test/basecamp/services/checkins_service_test.rb +++ b/ruby/test/basecamp/services/checkins_service_test.rb @@ -32,14 +32,9 @@ def sample_question(id: 1, title: "What did you work on today?") } end - def sample_answer(id: 1, content: "

Making great progress!

") - { - "id" => id, - "content" => content, - "content_attachments" => [], - "creator" => { "id" => 1, "name" => "Test User" }, - "created_at" => "2024-01-01T00:00:00Z" - } + def sample_answer(id: nil, content: nil) + fixture = load_fixture("checkins/answer.json") + fixture.merge "id" => id || fixture["id"], "content" => content || fixture["content"] end def test_get_questionnaire @@ -107,7 +102,7 @@ def test_list_answers answers = @account.checkins.list_answers(question_id: 200).to_a assert_equal 2, answers.length - assert_equal "

Making great progress!

", answers[0]["content"] + assert_equal load_fixture("checkins/answer.json")["content"], answers[0]["content"] end def test_get_answer @@ -117,7 +112,7 @@ def test_get_answer answer = @account.checkins.get_answer(answer_id: 200) assert_equal 200, answer["id"] - assert_equal "

Making great progress!

", answer["content"] + assert_equal load_fixture("checkins/answer.json")["content"], answer["content"] end def test_create_answer diff --git a/ruby/test/basecamp/services/client_approvals_service_test.rb b/ruby/test/basecamp/services/client_approvals_service_test.rb index e3073adeb..7a7487dd0 100644 --- a/ruby/test/basecamp/services/client_approvals_service_test.rb +++ b/ruby/test/basecamp/services/client_approvals_service_test.rb @@ -14,15 +14,9 @@ def setup @account = create_account_client(account_id: "12345") end - def sample_approval(id: 1, subject: "Design Review") - { - "id" => id, - "subject" => subject, - "approval_status" => "pending", - "content" => "

Please review the attached designs.

", - "content_attachments" => [], - "created_at" => "2024-01-01T00:00:00Z" - } + def sample_approval(id: nil, subject: nil) + fixture = load_fixture("client_approvals/get.json") + fixture.merge "id" => id || fixture["id"], "subject" => subject || fixture["subject"] end def test_list_approvals @@ -32,7 +26,7 @@ def test_list_approvals approvals = @account.client_approvals.list.to_a assert_equal 2, approvals.length - assert_equal "Design Review", approvals[0]["subject"] + assert_equal "New logo for the website", approvals[0]["subject"] assert_equal "Budget Approval", approvals[1]["subject"] end @@ -43,7 +37,7 @@ def test_get_approval approval = @account.client_approvals.get(approval_id: 200) assert_equal 200, approval["id"] - assert_equal "Design Review", approval["subject"] - assert_equal "pending", approval["approval_status"] + assert_equal "New logo for the website", approval["subject"] + assert_equal "approved", approval["approval_status"] end end diff --git a/ruby/test/basecamp/services/client_correspondences_service_test.rb b/ruby/test/basecamp/services/client_correspondences_service_test.rb index fadea7ea5..f06dfb914 100644 --- a/ruby/test/basecamp/services/client_correspondences_service_test.rb +++ b/ruby/test/basecamp/services/client_correspondences_service_test.rb @@ -14,15 +14,9 @@ def setup @account = create_account_client(account_id: "12345") end - def sample_correspondence(id: 1, subject: "Project Update") - { - "id" => id, - "subject" => subject, - "content" => "

Here is the latest update on the project.

", - "content_attachments" => [], - "replies_count" => 3, - "created_at" => "2024-01-01T00:00:00Z" - } + def sample_correspondence(id: nil, subject: nil) + fixture = load_fixture("client_correspondences/get.json") + fixture.merge "id" => id || fixture["id"], "subject" => subject || fixture["subject"] end def test_list_correspondences @@ -32,7 +26,7 @@ def test_list_correspondences correspondences = @account.client_correspondences.list.to_a assert_equal 2, correspondences.length - assert_equal "Project Update", correspondences[0]["subject"] + assert_equal "Project kickoff!", correspondences[0]["subject"] assert_equal "Invoice Query", correspondences[1]["subject"] end @@ -43,7 +37,7 @@ def test_get_correspondence correspondence = @account.client_correspondences.get(correspondence_id: 200) assert_equal 200, correspondence["id"] - assert_equal "Project Update", correspondence["subject"] - assert_equal 3, correspondence["replies_count"] + assert_equal "Project kickoff!", correspondence["subject"] + assert_equal 5, correspondence["replies_count"] end end diff --git a/ruby/test/basecamp/services/client_replies_service_test.rb b/ruby/test/basecamp/services/client_replies_service_test.rb index c56ddef75..74e6f6111 100644 --- a/ruby/test/basecamp/services/client_replies_service_test.rb +++ b/ruby/test/basecamp/services/client_replies_service_test.rb @@ -14,14 +14,9 @@ def setup @account = create_account_client(account_id: "12345") end - def sample_reply(id: 1, content: "

Thank you for the update!

") - { - "id" => id, - "content" => content, - "content_attachments" => [], - "creator" => { "id" => 1, "name" => "Client User" }, - "created_at" => "2024-01-01T00:00:00Z" - } + def sample_reply(id: nil, content: nil) + fixture = load_fixture("client_replies/get.json") + fixture.merge "id" => id || fixture["id"], "content" => content || fixture["content"] end def test_list_replies @@ -31,7 +26,7 @@ def test_list_replies replies = @account.client_replies.list(recording_id: 200).to_a assert_equal 2, replies.length - assert_equal "

Thank you for the update!

", replies[0]["content"] + assert_equal load_fixture("client_replies/get.json")["content"], replies[0]["content"] assert_equal "

Looking forward to it!

", replies[1]["content"] end @@ -42,7 +37,7 @@ def test_get_reply reply = @account.client_replies.get(recording_id: 200, reply_id: 300) assert_equal 300, reply["id"] - assert_equal "

Thank you for the update!

", reply["content"] - assert_equal "Client User", reply["creator"]["name"] + assert_equal load_fixture("client_replies/get.json")["content"], reply["content"] + assert_equal "Annie Bryan", reply["creator"]["name"] end end diff --git a/ruby/test/basecamp/services/comments_service_test.rb b/ruby/test/basecamp/services/comments_service_test.rb index 8e2d98e8e..888029a33 100644 --- a/ruby/test/basecamp/services/comments_service_test.rb +++ b/ruby/test/basecamp/services/comments_service_test.rb @@ -15,14 +15,9 @@ def setup @account = create_account_client(account_id: "12345") end - def sample_comment(id: 1, content: "

Great work!

") - { - "id" => id, - "content" => content, - "content_attachments" => [], - "creator" => { "id" => 1, "name" => "Test User" }, - "created_at" => "2024-01-01T00:00:00Z" - } + def sample_comment(id: nil, content: nil) + fixture = load_fixture("comments/get.json") + fixture.merge "id" => id || fixture["id"], "content" => content || fixture["content"] end def test_list_comments @@ -32,7 +27,7 @@ def test_list_comments comments = @account.comments.list(recording_id: 200).to_a assert_equal 2, comments.length - assert_equal "

Great work!

", comments[0]["content"] + assert_equal load_fixture("comments/get.json")["content"], comments[0]["content"] assert_equal "

I agree!

", comments[1]["content"] end @@ -43,7 +38,7 @@ def test_get_comment comment = @account.comments.get(comment_id: 200) assert_equal 200, comment["id"] - assert_equal "

Great work!

", comment["content"] + assert_equal load_fixture("comments/get.json")["content"], comment["content"] end def test_create_comment diff --git a/ruby/test/basecamp/services/documents_service_test.rb b/ruby/test/basecamp/services/documents_service_test.rb index 95648749e..13855586a 100644 --- a/ruby/test/basecamp/services/documents_service_test.rb +++ b/ruby/test/basecamp/services/documents_service_test.rb @@ -14,16 +14,9 @@ def setup @account = create_account_client(account_id: "12345") end - def sample_document(id: 1, title: "Meeting Notes") - { - "id" => id, - "title" => title, - "content" => "

Notes from today's meeting...

", - "content_attachments" => [], - "status" => "active", - "comments_count" => 2, - "created_at" => "2024-01-01T00:00:00Z" - } + def sample_document(id: nil, title: nil) + fixture = load_fixture("documents/get.json") + fixture.merge "id" => id || fixture["id"], "title" => title || fixture["title"] end def test_list_documents @@ -33,7 +26,7 @@ def test_list_documents documents = @account.documents.list(vault_id: 200).to_a assert_equal 2, documents.length - assert_equal "Meeting Notes", documents[0]["title"] + assert_equal "Project Overview", documents[0]["title"] assert_equal "Project Plan", documents[1]["title"] end @@ -44,7 +37,7 @@ def test_get_document document = @account.documents.get(document_id: 200) assert_equal 200, document["id"] - assert_equal "Meeting Notes", document["title"] + assert_equal "Project Overview", document["title"] end def test_create_document diff --git a/ruby/test/basecamp/services/forwards_service_test.rb b/ruby/test/basecamp/services/forwards_service_test.rb index fbec6487a..74310889e 100644 --- a/ruby/test/basecamp/services/forwards_service_test.rb +++ b/ruby/test/basecamp/services/forwards_service_test.rb @@ -22,25 +22,14 @@ def sample_inbox(id: 1) } end - def sample_forward(id: 1, subject: "Client Question") - { - "id" => id, - "subject" => subject, - "from" => "client@example.com", - "content" => "

I have a question about the project.

", - "content_attachments" => [], - "created_at" => "2024-01-01T00:00:00Z" - } + def sample_forward(id: nil, subject: nil) + fixture = load_fixture("forwards/get.json") + fixture.merge "id" => id || fixture["id"], "subject" => subject || fixture["subject"] end - def sample_reply(id: 1, content: "

Thanks for reaching out!

") - { - "id" => id, - "content" => content, - "content_attachments" => [], - "creator" => { "id" => 1, "name" => "Test User" }, - "created_at" => "2024-01-01T00:00:00Z" - } + def sample_reply(id: nil, content: nil) + fixture = load_fixture("forwards/reply_get.json") + fixture.merge "id" => id || fixture["id"], "content" => content || fixture["content"] end def test_get_inbox @@ -60,7 +49,7 @@ def test_list_forwards forwards = @account.forwards.list(inbox_id: 200).to_a assert_equal 2, forwards.length - assert_equal "Client Question", forwards[0]["subject"] + assert_equal "Project proposal from client", forwards[0]["subject"] assert_equal "Another Email", forwards[1]["subject"] end @@ -71,7 +60,7 @@ def test_get_forward forward = @account.forwards.get(forward_id: 200) assert_equal 200, forward["id"] - assert_equal "Client Question", forward["subject"] + assert_equal "Project proposal from client", forward["subject"] assert_equal "client@example.com", forward["from"] end @@ -82,7 +71,7 @@ def test_list_replies replies = @account.forwards.list_replies(forward_id: 200).to_a assert_equal 2, replies.length - assert_equal "

Thanks for reaching out!

", replies[0]["content"] + assert_equal load_fixture("forwards/reply_get.json")["content"], replies[0]["content"] end def test_get_reply @@ -92,7 +81,7 @@ def test_get_reply reply = @account.forwards.get_reply(forward_id: 200, reply_id: 300) assert_equal 300, reply["id"] - assert_equal "

Thanks for reaching out!

", reply["content"] + assert_equal load_fixture("forwards/reply_get.json")["content"], reply["content"] end def test_create_reply diff --git a/ruby/test/basecamp/services/messages_service_test.rb b/ruby/test/basecamp/services/messages_service_test.rb index 4191ed5da..27969183c 100644 --- a/ruby/test/basecamp/services/messages_service_test.rb +++ b/ruby/test/basecamp/services/messages_service_test.rb @@ -16,16 +16,9 @@ def setup @account = create_account_client(account_id: "12345") end - def sample_message(id: 789, subject: "Test Message") - { - "id" => id, - "subject" => subject, - "content" => "

Message content

", - "content_attachments" => [], - "status" => "active", - "created_at" => "2024-01-01T00:00:00Z", - "creator" => { "id" => 1, "name" => "Test User" } - } + def sample_message(id: nil, subject: nil) + fixture = load_fixture("messages/get.json") + fixture.merge "id" => id || fixture["id"], "subject" => subject || fixture["subject"] end def test_list @@ -35,7 +28,7 @@ def test_list result = @account.messages.list(board_id: 456).to_a assert_equal 2, result.length - assert_equal "Test Message", result[0]["subject"] + assert_equal "We won Leto!", result[0]["subject"] end def test_list_with_sort_and_direction @@ -53,12 +46,12 @@ def test_list_with_sort_and_direction def test_get # Generated service: /messages/{id} without .json - stub_get("/12345/messages/789", response_body: sample_message) + stub_get("/12345/messages/789", response_body: sample_message(id: 789)) result = @account.messages.get(message_id: 789) assert_equal 789, result["id"] - assert_equal "Test Message", result["subject"] + assert_equal "We won Leto!", result["subject"] end def test_create diff --git a/ruby/test/basecamp/services/recordings_service_test.rb b/ruby/test/basecamp/services/recordings_service_test.rb index 0f7509d35..b5cbc0d1f 100644 --- a/ruby/test/basecamp/services/recordings_service_test.rb +++ b/ruby/test/basecamp/services/recordings_service_test.rb @@ -19,17 +19,13 @@ def setup @account = create_account_client(account_id: "12345") end - def sample_recording(id: 456, title: "Test Recording") - { - "id" => id, - "title" => title, - "status" => "active", - "type" => "Todo", - # The generic recording projection carries the matching type's rich-text - # companion array; a Todo recording surfaces description_attachments. - "description_attachments" => [], - "created_at" => "2024-01-01T00:00:00Z" - } + # Sourced from the shared recordings/get.json fixture (the validated source of + # truth). It is a Message recording, so the rich-text companion array it + # carries is content_attachments (one inline file); description_attachments is + # absent for this type. + def sample_recording(id: nil, title: nil) + fixture = load_fixture("recordings/get.json") + fixture.merge "id" => id || fixture["id"], "title" => title || fixture["title"] end def test_list @@ -41,7 +37,7 @@ def test_list result = @account.recordings.list(type: "Todo").to_a assert_equal 2, result.length - assert_equal "Test Recording", result[0]["title"] + assert_equal "We won Leto!", result[0]["title"] end def test_list_with_filters @@ -58,14 +54,16 @@ def test_list_with_filters def test_get # Generated service: /recordings/{id} without .json - stub_get("/12345/recordings/456", response_body: sample_recording) + stub_get("/12345/recordings/456", response_body: sample_recording(id: 456)) result = @account.recordings.get(recording_id: 456) assert_equal 456, result["id"] - assert_equal "Test Recording", result["title"] - # The optional projection array surfaces on the matching-type recording. - assert_equal [], result["description_attachments"] + assert_equal "We won Leto!", result["title"] + # This Message recording surfaces content_attachments (one inline file); the + # description_attachments projection is absent for this type. + assert_equal 1, result["content_attachments"].length + assert_nil result["description_attachments"] end def test_archive diff --git a/ruby/test/basecamp/services/schedules_service_test.rb b/ruby/test/basecamp/services/schedules_service_test.rb index 0021dd630..59b45c390 100644 --- a/ruby/test/basecamp/services/schedules_service_test.rb +++ b/ruby/test/basecamp/services/schedules_service_test.rb @@ -24,15 +24,9 @@ def sample_schedule(id: 456) } end - def sample_entry(id: 789, summary: "Team Meeting") - { - "id" => id, - "summary" => summary, - "description_attachments" => [], - "starts_at" => "2024-12-15T09:00:00Z", - "ends_at" => "2024-12-15T10:00:00Z", - "all_day" => false - } + def sample_entry(id: nil, summary: nil) + fixture = load_fixture("schedules/entry_get.json") + fixture.merge "id" => id || fixture["id"], "summary" => summary || fixture["summary"] end def test_get @@ -51,16 +45,16 @@ def test_list_entries result = @account.schedules.list_entries(schedule_id: 456).to_a assert_equal 2, result.length - assert_equal "Team Meeting", result[0]["summary"] + assert_equal "Project Kickoff Meeting", result[0]["summary"] end def test_get_entry - stub_get("/12345/schedule_entries/789", response_body: sample_entry) + stub_get("/12345/schedule_entries/789", response_body: sample_entry(id: 789)) result = @account.schedules.get_entry(entry_id: 789) assert_equal 789, result["id"] - assert_equal "Team Meeting", result["summary"] + assert_equal "Project Kickoff Meeting", result["summary"] end def test_create_entry diff --git a/ruby/test/test_helper.rb b/ruby/test/test_helper.rb index 22fa4c100..386a26ccd 100644 --- a/ruby/test/test_helper.rb +++ b/ruby/test/test_helper.rb @@ -184,6 +184,16 @@ def stub_delete(path, status: 204) .to_return(status: status, body: "") end + # Load a shared JSON fixture (the validated source of truth) as string-keyed + # hashes. test_helper.rb lives at ruby/test/, so "../../spec/fixtures" reaches + # the repo-root spec/fixtures directory. Read as explicit UTF-8 so a non-UTF-8 + # process locale (LC_ALL=C) doesn't choke on the emoji/non-ASCII bytes several + # fixtures carry. + def load_fixture(relative_path) + path = File.expand_path("../../spec/fixtures/#{relative_path}", __dir__) + JSON.parse(File.read(path, encoding: "UTF-8")) + end + # Sample project data def sample_project(id: 123, name: "Test Project") { diff --git a/scripts/check-fixture-coverage.rb b/scripts/check-fixture-coverage.rb new file mode 100755 index 000000000..a0ba6c59c --- /dev/null +++ b/scripts/check-fixture-coverage.rb @@ -0,0 +1,622 @@ +#!/usr/bin/env ruby +# frozen_string_literal: true + +# Fixture-completeness guard. +# +# Validates spec/fixtures/manifest.yaml: +# 1. Every `targets` entry resolves — fixture exists, JSON pointer selects a +# concrete instance, the named schema is a real component — and the selected +# instance validates against its schema: every required field present and +# non-null (unless the field schema is nullable), and every present value's +# JSON type matches the declared type (a float is accepted for an integer +# field only when mathematically integral; a null array element fails a +# non-nullable item schema). Validation is composition-aware — it merges +# `$ref` (including 3.1 `$ref`-with-siblings) and `allOf`, so a composed +# target whose required fields live in a branch is still enforced. +# 2. The coverage invariant: every schema in `covered_schemas` has >= 1 active +# target that resolves to a concrete instance whose declared root schema is +# that component (the concrete-instance rule — transitive reachability and +# empty arrays do not count; an object component wants a Hash, an array +# component a non-empty Array). +# 3. Inventory completeness: every RichTextAttachment-emitting component schema +# (the #408 class this guard exists to protect) must be either covered or +# excluded — so the inventory can't silently shrink by dropping a schema. +# Emitter discovery follows `$ref` (with siblings), traverses component-level +# `allOf`/`anyOf`/`oneOf`, and resolves aliased array items, guarding cycles. +# 4. `excluded_schemas` entries each carry a reason + tracking issue, name real +# components, and do not overlap `covered_schemas`. +# +# Uses conformance/runner/ruby/schema-walker.rb only for `find_response_schema` +# (operation-entry response-schema lookup). The required/type/nullability walk is +# implemented here because it must be composition-aware, which the walker is not. +# Wired into `make check` in the scripts/validate-api-gaps.rb style (stdlib only). +# +# Paths default to the repo layout but honour FIXTURE_MANIFEST / FIXTURE_OPENAPI +# / FIXTURE_DIR env overrides so the negative-case self-test +# (scripts/test-check-fixture-coverage.rb) can point it at crafted inputs. +# +# --- Scope: a deliberately PARTIAL structural validator ------------------------ +# +# This is NOT a complete JSON-Schema / OpenAPI validator. It validates the subset +# that keeps a fixture structurally faithful to the generated types: +# * required-field presence +# * declared types (with integer/number integrality) and nullability +# * arrays and array-element typing/nullability +# * $ref (incl. 3.1 $ref-with-siblings) and allOf conjunction (required unioned; +# duplicate properties and array `items` conjoined; type constraints +# intersected; nullability = every part permits null) +# * anyOf/oneOf as AT-LEAST-ONE (the value must satisfy some branch; a group is +# nullable only if a branch is) +# +# Intentionally NOT implemented (out of scope unless a case is reached by the +# actual generated schemas): exact-one `oneOf` selection, `enum`, `const`, +# discriminators, `pattern`, `format`, numeric bounds, `additionalProperties`, +# `uniqueItems`, and other assertion keywords. Findings about these are declined +# unless they affect the current generated `openapi.json` or this documented +# subset. + +require "json" +require "yaml" +require "set" + +require_relative "../conformance/runner/ruby/schema-walker" + +PROJECT_ROOT = File.expand_path("..", __dir__) +MANIFEST_FILE = ENV.fetch("FIXTURE_MANIFEST", File.join(PROJECT_ROOT, "spec/fixtures/manifest.yaml")) +FIXTURES_DIR = ENV.fetch("FIXTURE_DIR", File.join(PROJECT_ROOT, "spec/fixtures")) +OPENAPI_FILE = ENV.fetch("FIXTURE_OPENAPI", File.join(PROJECT_ROOT, "openapi.json")) + +# Read text as UTF-8 regardless of the process locale (LC_ALL=C would otherwise +# read as US-ASCII and choke on the spec's UTF-8 bytes). +def read_utf8(path) + File.read(path, encoding: "UTF-8") +end + +errors = [] + +def fail_with(errors, message) + errors << message +end + +# RFC 6901 JSON pointer resolution. Returns [value, found]. "" selects the whole +# document. Escapes: `~0`->`~`, `~1`->`/`; a `~` not followed by 0/1 is invalid. +# Array indices must be "0" or a non-zero-leading run of digits (§4) — "01" is +# malformed and does NOT silently resolve element 1. Missing keys / out-of-range +# indices / non-container traversal / bad escapes -> [nil, false]. +def resolve_pointer(doc, pointer) + return [doc, true] if pointer.nil? || pointer.empty? + return [nil, false] unless pointer.start_with?("/") + + current = doc + pointer.split("/", -1).drop(1).each do |raw| + return [nil, false] if raw.match?(/~(?![01])/) # invalid escape (e.g. ~2, trailing ~) + + token = raw.gsub("~1", "/").gsub("~0", "~") + case current + when Hash + return [nil, false] unless current.key?(token) + + current = current[token] + when Array + return [nil, false] unless token == "0" || token.match?(/\A[1-9]\d*\z/) + + idx = token.to_i + return [nil, false] if idx >= current.length + + current = current[idx] + else + return [nil, false] + end + end + [current, true] +end + +# --- Schema helpers ------------------------------------------------------------ + +# Ruby name of a `$ref`, or nil. +def ref_name(schema) + return nil unless schema.is_a?(Hash) && schema["$ref"].is_a?(String) + + schema["$ref"].match(%r{/components/schemas/(.+)\z})&.captures&.first +end + +# Returns [Set(declared non-null json-type strings), nullable?] for a single +# schema node (no traversal). Handles OpenAPI 3.1 null-union (`type:[X,"null"]`) +# and 3.0 `nullable:true`. An empty set means the node declares no type. +def allowed_types(schema) + return [Set.new, false] unless schema.is_a?(Hash) + + t = schema["type"] + case t + when Array + members = t.compact + non_null = members - ["null"] + # A union of only ["null"] still constrains the value to null — keep "null" + # as the type so a non-null value fails (an empty set would be unconstrained). + types = non_null.empty? ? Set.new(["null"]) : Set.new(non_null) + [types, members.include?("null")] + when String + # OpenAPI 3.1 scalar null type: the value must BE null — a real "null" type + # constraint (so a non-null value fails), and it is nullable. Returning an + # empty type set would wrongly impose no constraint at all. + return [Set.new(["null"]), true] if t == "null" + + [Set.new([t]), schema["nullable"] == true] + else + [Set.new, schema["nullable"] == true] + end +end + +def json_type(value) + case value + when Hash then "object" + when Array then "array" + when String then "string" + when true, false then "boolean" + when Integer then "integer" + when Float then "number" + else "null" + end +end + +# True when `value`'s JSON type satisfies the declared `types`. integer/number +# interchange with one guard: a float supplied for an integer-only field passes +# only when it is mathematically integral (so FlexInt `1024.0` passes but `1.5` +# fails). +def type_matches?(types, value) + return true if types.empty? + + actual = json_type(value) + return true if types.include?(actual) + + if actual == "number" && types.include?("integer") && !types.include?("number") + return value.is_a?(Float) && value.finite? && value == value.truncate + end + return true if actual == "integer" && types.include?("number") + + false +end + +# Merges a schema's effective constraints across `$ref` (including 3.1 +# `$ref`-with-siblings) and `allOf`, returning +# [required(Array), properties(Hash), types(Set), nullable(bool), items(schema), +# alt_groups(Array of branch-arrays), type_sets(Array of Sets)]. +# `allOf` is a conjunction: required is unioned, and properties AND array `items` +# constrained by more than one branch are conjoined (allOf-wrapped) so a value +# must satisfy all of them; `type_sets` holds each part's declared type-set (a +# value must match every one). `anyOf`/`oneOf` are alternatives: their branches +# are NOT merged (that would over-require) but each group is collected so +# validation can require at least one branch to match — including groups +# inherited through `$ref` and `allOf`. `visited` (component names) + depth guard +# terminate reference/composition cycles. +def merged_constraints(schema, components, visited = Set.new, depth = 0) + req = [] + props = {} + types = Set.new # union of all declared types (for messages + concrete_for?) + type_sets = [] # per-conjunctive-part declared type-sets — a value must + # satisfy EVERY one (allOf/$ref are a conjunction, so their + # type constraints INTERSECT, not union). + # `$ref`(+siblings) and allOf form a CONJUNCTION: null is allowed only if every + # part allows it, so a part that imposes a non-nullable type FORBIDS null. We + # accumulate `forbids_null` (OR) and return its negation as the nullable flag. + forbids_null = false + items = nil + alt_groups = [] + return [req, props, types, true, items, alt_groups, type_sets] if depth > 40 || !schema.is_a?(Hash) + + # When the same property (or array `items`) is constrained by more than one + # conjunctive part (e.g. declared in two allOf branches), conjoin the schemas + # so the value must satisfy ALL of them — not just the first seen. + add_prop = lambda do |k, v| + props[k] = props.key?(k) ? { "allOf" => [props[k], v] } : v + end + add_items = lambda do |i| + items = items ? { "allOf" => [items, i] } : i + end + + absorb = lambda do |sub| + r2, p2, t2, sub_nullable, i2, a2, ts2 = merged_constraints(sub, components, visited, depth + 1) + req.concat(r2) + p2.each { |k, v| add_prop.call(k, v) } + types.merge(t2) + type_sets.concat(ts2) + forbids_null ||= !sub_nullable + add_items.call(i2) if i2 + alt_groups.concat(a2) + end + + name = ref_name(schema) + if name && !visited.include?(name) + visited << name + absorb.call(components[name]) + # fall through to local keywords (OpenAPI 3.1 permits $ref siblings) + end + + t, nn = allowed_types(schema) + types.merge(t) + type_sets << t unless t.empty? + # A node that imposes a concrete type but does not permit null forbids null. + forbids_null ||= (!t.empty? && !nn) + (schema["properties"] || {}).each { |k, v| add_prop.call(k, v) } + (schema["required"] || []).each { |r| req << r } + add_items.call(schema["items"]) if schema["items"] + (schema["allOf"] || []).each { |sub| absorb.call(sub) } + %w[anyOf oneOf].each do |key| + alt_groups << schema[key] if schema[key].is_a?(Array) && !schema[key].empty? + end + + # An anyOf/oneOf group (local or inherited via $ref/allOf) permits null only if + # at least one branch does; if every branch forbids null, the group forbids it + # (the value must satisfy some branch). Conjoin that with the surrounding + # $ref/allOf constraints. Branch nullability is computed with a FRESH visited + # set so outer traversal state can't short-circuit it. + alt_groups.each do |branches| + group_allows_null = branches.any? do |branch| + _, _, _, branch_nullable, = merged_constraints(branch, components, Set.new, depth + 1) + branch_nullable + end + forbids_null ||= !group_allows_null + end + + [req, props, types, !forbids_null, items, alt_groups, type_sets] +end + +# Composition-aware validation of `value` against `schema`. Reports +# (path-tagged) errors for a missing required field, a required field present as +# null against a non-nullable schema, a null array element against a non-nullable +# item schema, and a present value whose JSON type contradicts the declared type. +# Optional object-field nulls are tolerated: the Smithy-derived OpenAPI +# under-marks some nullable optionals (e.g. Person.bio/location are `type:string` +# yet the wire sends null), so flagging them would be a false positive. +def instance_errors(prefix, value, schema, components, depth = 0) + return [] if depth > 60 + return [] if value.nil? # optional-null tolerated; required-/element-null handled in context + + req, props, _types, _nullable, items, alt_groups, type_sets = merged_constraints(schema, components) + + # The value must satisfy EVERY conjunctive part's declared type (allOf/$ref + # intersect their type constraints — a value matching only one contradictory + # branch fails). + type_sets.each do |ts| + next if type_matches?(ts, value) + + label = prefix.empty? ? "(root)" : prefix + return ["#{label}: expected #{ts.to_a.sort.join('|')}, got #{json_type(value)}"] + end + + errs = [] + + # anyOf/oneOf: the value must satisfy at least one branch of each group + # (oneOf is validated as "at least one" — enforcing exactly-one would need full + # discriminator/enum/const validation to avoid false positives). + alt_groups.each do |branches| + next if branches.any? { |branch| instance_errors(prefix, value, branch, components, depth + 1).empty? } + + label = prefix.empty? ? "(root)" : prefix + errs << "#{label}: value matches none of the #{branches.length} allowed alternatives (anyOf/oneOf)" + end + + if value.is_a?(Hash) + req.uniq.each do |rk| + field = prefix.empty? ? rk : "#{prefix}/#{rk}" + if !value.key?(rk) + errs << "missing required field `#{field}`" + elsif value[rk].nil? + _, _, _, field_nullable, = merged_constraints(props[rk] || {}, components) + errs << "#{field}: required field is null but its schema is not nullable" unless field_nullable + end + end + value.each do |k, v| + next unless props.key?(k) + + child = prefix.empty? ? k : "#{prefix}/#{k}" + errs.concat(instance_errors(child, v, props[k], components, depth + 1)) + end + elsif value.is_a?(Array) && items + _, _, _, item_nullable, = merged_constraints(items, components) + value.each_with_index do |item, i| + ip = "#{prefix}[#{i}]" + if item.nil? + errs << "#{ip}: null array element but the item schema is not nullable" unless item_nullable + next + end + errs.concat(instance_errors(ip, item, items, components, depth + 1)) + end + end + + errs +end + +def concrete_for?(schema_name, instance, components) + _, _, types, = merged_constraints({ "$ref" => "#/components/schemas/#{schema_name}" }, components) + if types.include?("array") + instance.is_a?(Array) && !instance.empty? + else + instance.is_a?(Hash) + end +end + +# True when `schema` denotes (through `$ref` chains + siblings and +# allOf/anyOf/oneOf composition) the RichTextAttachment component — so an alias +# (`{$ref: SomeAlias}` -> RichTextAttachment) or a composed item schema is still +# recognized. `visited` (component names) terminates ref cycles. +def rich_text_attachment?(schema, components, visited = Set.new, depth = 0) + return false if depth > 40 || !schema.is_a?(Hash) + + name = ref_name(schema) + if name + return true if name == "RichTextAttachment" + + unless visited.include?(name) + visited << name + return true if rich_text_attachment?(components[name], components, visited, depth + 1) + end + # fall through to local keywords ($ref siblings) + end + + %w[allOf anyOf oneOf].each do |key| + (schema[key] || []).each do |sub| + return true if rich_text_attachment?(sub, components, visited, depth + 1) + end + end + false +end + +# True when `schema` is (or composes/refs) an array whose items resolve to the +# RichTextAttachment component. Follows `$ref` (with siblings) and +# allOf/anyOf/oneOf on both the array schema and its items; `visited` guards +# cycles. +def references_rich_text_array?(schema, components, visited = Set.new, depth = 0) + return false if depth > 40 || !schema.is_a?(Hash) + + name = ref_name(schema) + if name && !visited.include?(name) + visited << name + return true if references_rich_text_array?(components[name], components, visited, depth + 1) + # fall through to local keywords ($ref siblings) + end + + t = schema["type"] + if (t == "array" || (t.is_a?(Array) && t.include?("array"))) && schema["items"] + return true if rich_text_attachment?(schema["items"], components) + end + + %w[allOf anyOf oneOf].each do |key| + (schema[key] || []).each do |sub| + return true if references_rich_text_array?(sub, components, visited, depth + 1) + end + end + false +end + +# Collects every property schema of a component, following `$ref` (with siblings) +# and traversing component-level allOf/anyOf/oneOf so composed/inherited +# properties are seen. `visited` (component names) guards ref cycles. +def collect_property_schemas(schema, components, out, visited = Set.new, depth = 0) + return if depth > 40 || !schema.is_a?(Hash) + + name = ref_name(schema) + if name && !visited.include?(name) + visited << name + collect_property_schemas(components[name], components, out, visited, depth + 1) + # fall through to local keywords ($ref siblings) + end + + (schema["properties"] || {}).each_value { |ps| out << ps } + %w[allOf anyOf oneOf].each do |key| + (schema[key] || []).each do |sub| + collect_property_schemas(sub, components, out, visited, depth + 1) + end + end +end + +# A whole-component alias — a schema that is only a `$ref` to another component +# (bare annotations aside). It introduces no decode surface of its own, so it is +# NOT an independent emitter: covering its target covers it, and if that target +# is itself an emitter it is caught directly. Skipping these keeps the emitter +# set to the concrete decode types (the ~40 `*ResponseContent` response +# envelopes are pure aliases of already-covered components). A `$ref` WITH local +# structure (3.1 siblings) is not a pure alias and is still examined. +def pure_ref_alias?(schema) + schema.is_a?(Hash) && schema.key?("$ref") && + (schema.keys - %w[$ref description title]).empty? +end + +def rich_text_emitters(components) + components.select do |_name, schema| + next false unless schema.is_a?(Hash) + next false if pure_ref_alias?(schema) + + props = [] + collect_property_schemas(schema, components, props) + props.any? { |ps| references_rich_text_array?(ps, components) } + end.keys +end + +# --- Load ----------------------------------------------------------------------- + +unless File.file?(MANIFEST_FILE) + warn "ERROR: manifest not found at #{MANIFEST_FILE}" + exit 2 +end +unless File.file?(OPENAPI_FILE) + warn "ERROR: openapi.json not found at #{OPENAPI_FILE}" + exit 2 +end + +manifest = YAML.safe_load(read_utf8(MANIFEST_FILE)) +walker = Basecamp::Conformance::SchemaWalker.new(OPENAPI_FILE) +openapi = JSON.parse(read_utf8(OPENAPI_FILE)) +components = openapi.dig("components", "schemas") || {} + +targets = manifest["targets"] || [] +covered = manifest["covered_schemas"] || {} +excluded = manifest["excluded_schemas"] || {} + +fixture_cache = {} +load_fixture = lambda do |rel| + fixture_cache[rel] ||= begin + path = File.join(FIXTURES_DIR, rel) + File.file?(path) ? JSON.parse(read_utf8(path)) : :missing + end +end + +# --- 1. Targets ---------------------------------------------------------------- + +by_id = {} +resolved_root = {} + +targets.each_with_index do |entry, i| + id = entry["id"] + if id.nil? || id.to_s.empty? + fail_with(errors, "targets[#{i}]: missing `id`") + next + end + if by_id.key?(id) + fail_with(errors, "target `#{id}`: duplicate id") + next + end + by_id[id] = entry + + operation = entry["operation"] + fixture_rel = entry["fixture"] + pointer = entry["pointer"] || "" + schema_name = entry["schema"] + + if operation && schema_name + fail_with(errors, "target `#{id}`: `operation` and `schema` are mutually exclusive") + next + end + if operation && !pointer.empty? + fail_with(errors, + "target `#{id}`: operation entries validate the whole response body — " \ + "a non-root `pointer` (#{pointer}) is not allowed (use a pointer entry with an explicit schema)") + next + end + + schema, root_schema_name = + if operation + s = walker.find_response_schema(operation) + unless s + fail_with(errors, "target `#{id}`: no 2xx/default response schema for operation `#{operation}`") + next + end + m = s.is_a?(Hash) ? s["$ref"].to_s.match(%r{/components/schemas/(.+)\z}) : nil + [s, m && m[1]] + else + unless schema_name + fail_with(errors, "target `#{id}`: pointer entry needs an explicit `schema`") + next + end + unless components.key?(schema_name) + fail_with(errors, "target `#{id}`: unknown component schema `#{schema_name}`") + next + end + [{ "$ref" => "#/components/schemas/#{schema_name}" }, schema_name] + end + + unless fixture_rel + fail_with(errors, "target `#{id}`: missing `fixture`") + next + end + doc = load_fixture.call(fixture_rel) + if doc == :missing + fail_with(errors, "target `#{id}`: fixture not found: spec/fixtures/#{fixture_rel}") + next + end + + instance, found = resolve_pointer(doc, pointer) + unless found + fail_with(errors, "target `#{id}`: JSON pointer `#{pointer}` does not resolve in #{fixture_rel}") + next + end + if instance.nil? + fail_with(errors, "target `#{id}`: pointer `#{pointer}` resolves to null (not a concrete instance) in #{fixture_rel}") + next + end + + concrete = root_schema_name ? concrete_for?(root_schema_name, instance, components) : instance.is_a?(Hash) + resolved_root[id] = { name: root_schema_name, concrete: concrete } + + where = pointer.empty? ? fixture_rel : "#{fixture_rel} at pointer `#{pointer}`" + instance_errors("", instance, schema, components).each do |msg| + fail_with(errors, "target `#{id}`: #{where} #{msg}") + end +end + +# --- 2. Coverage invariant ----------------------------------------------------- + +covered.each do |schema_name, ids| + unless components.key?(schema_name) + fail_with(errors, "covered_schemas: unknown component schema `#{schema_name}`") + next + end + ids = Array(ids) + if ids.empty? + fail_with(errors, "covered_schemas[`#{schema_name}`]: no target ids listed") + next + end + + concrete_rep = false + ids.each do |id| + unless by_id.key?(id) + fail_with(errors, "covered_schemas[`#{schema_name}`]: references unknown target `#{id}`") + next + end + root = resolved_root[id] + next unless root + + concrete_rep = true if root[:name] == schema_name && root[:concrete] + end + + unless concrete_rep + fail_with(errors, + "covered_schemas[`#{schema_name}`]: no active target resolves to a concrete " \ + "instance declared as `#{schema_name}` (concrete-instance rule)") + end +end + +# --- 3. Inventory completeness (non-self-erasing) ------------------------------ + +accounted = Set.new(covered.keys) | Set.new(excluded.keys) +rich_text_emitters(components).sort.each do |emitter| + next if accounted.include?(emitter) + + fail_with(errors, + "schema `#{emitter}` declares a RichTextAttachment array member but is neither " \ + "in covered_schemas nor excluded_schemas — the coverage inventory must account " \ + "for every rich-text emitter (add a covered target or an exclusion with a reason)") +end + +# --- 4. Exclusions ------------------------------------------------------------- + +excluded.each do |schema_name, meta| + unless components.key?(schema_name) + fail_with(errors, "excluded_schemas: unknown component schema `#{schema_name}`") + end + if covered.key?(schema_name) + fail_with(errors, "excluded_schemas[`#{schema_name}`]: schema is also in covered_schemas") + end + meta ||= {} + reason = meta["reason"] + if reason.nil? || reason.to_s.strip.empty? + fail_with(errors, "excluded_schemas[`#{schema_name}`]: missing `reason`") + end + issue = meta["issue"] + if issue.nil? || issue.to_s.strip.empty? + fail_with(errors, "excluded_schemas[`#{schema_name}`]: missing tracking `issue`") + end +end + +# --- Report -------------------------------------------------------------------- + +if errors.empty? + puts "==> Fixture coverage clean — #{covered.size} covered schemas, " \ + "#{targets.size} manifest targets, #{excluded.size} tracked exclusions, " \ + "#{rich_text_emitters(components).size} rich-text emitters accounted for" + exit 0 +else + warn "Fixture-coverage check failed:" + errors.sort.each { |e| warn " - #{e}" } + exit 1 +end diff --git a/scripts/check-fixture-coverage.sh b/scripts/check-fixture-coverage.sh new file mode 100755 index 000000000..3a7e3a5d2 --- /dev/null +++ b/scripts/check-fixture-coverage.sh @@ -0,0 +1,14 @@ +#!/usr/bin/env bash +# Fixture-completeness guard: validate spec/fixtures/manifest.yaml — every +# manifest target validates against its schema and every covered schema keeps a +# concrete active representative. See scripts/check-fixture-coverage.rb. +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" + +if ! command -v ruby >/dev/null 2>&1; then + echo "ERROR: ruby is required for check-fixture-coverage" >&2 + exit 2 +fi + +exec ruby "$SCRIPT_DIR/check-fixture-coverage.rb" diff --git a/scripts/test-check-fixture-coverage.rb b/scripts/test-check-fixture-coverage.rb new file mode 100755 index 000000000..8e54ecf3f --- /dev/null +++ b/scripts/test-check-fixture-coverage.rb @@ -0,0 +1,475 @@ +#!/usr/bin/env ruby +# frozen_string_literal: true + +# Negative-case self-test for scripts/check-fixture-coverage.rb. +# +# The guard's own `make check` run only exercises the VALID manifest. This test +# crafts each failure mode and asserts the guard rejects it (non-zero exit + an +# expected message fragment), driving the checker through its FIXTURE_MANIFEST / +# FIXTURE_OPENAPI / FIXTURE_DIR env overrides against tmp inputs. It also +# confirms the real manifest still passes (positive control). +# +# Run directly (`ruby scripts/test-check-fixture-coverage.rb`) or via +# `make check-fixture-coverage` (which runs it after the live check). + +require "json" +require "yaml" +require "tmpdir" +require "fileutils" +require "open3" +require "timeout" + +ROOT = File.expand_path("..", __dir__) +CHECKER = File.join(__dir__, "check-fixture-coverage.rb") +REAL_OPENAPI = File.join(ROOT, "openapi.json") +REAL_FIXTURES = File.join(ROOT, "spec/fixtures") +REAL_MANIFEST = File.join(REAL_FIXTURES, "manifest.yaml") + +def read_utf8(path) = File.read(path, encoding: "UTF-8") + +# Run the checker with env overrides; returns [combined_output, status]. +def run_checker(manifest:, openapi: REAL_OPENAPI, fixtures: REAL_FIXTURES) + env = { + "FIXTURE_MANIFEST" => manifest, + "FIXTURE_OPENAPI" => openapi, + "FIXTURE_DIR" => fixtures, + } + Open3.capture2e(env, "ruby", CHECKER) +end + +# Like run_checker but spawns with a retained PID so a hung child can actually be +# killed on timeout (Open3.capture2e's cleanup would block waiting for the +# child). Raises Timeout::Error after `seconds`, having KILLed and reaped the +# process. Returns [combined_output, Process::Status]. +def run_checker_killable(manifest:, openapi:, fixtures:, seconds: 30) + env = { "FIXTURE_MANIFEST" => manifest, "FIXTURE_OPENAPI" => openapi, "FIXTURE_DIR" => fixtures } + reader, writer = IO.pipe + pid = Process.spawn(env, "ruby", CHECKER, out: writer, err: writer) + writer.close + out = +"" + pump = Thread.new { out = reader.read } + status = nil + begin + Timeout.timeout(seconds) { _, status = Process.wait2(pid) } + rescue Timeout::Error + # Timeout can fire just after wait2 already reaped the child, so the kill/ + # reap may hit an already-gone process — tolerate ESRCH/ECHILD. + begin + Process.kill("KILL", pid) + Process.wait(pid) + rescue Errno::ESRCH, Errno::ECHILD + # already exited/reaped + end + raise + ensure + pump.join + reader.close + end + [out, status] +end + +failures = [] + +def expect_pass(failures, label, out, status) + return if status.success? + + failures << "#{label}: expected PASS but checker failed:\n#{out}" +end + +def expect_fail(failures, label, out, status, fragment) + if status.success? + failures << "#{label}: expected FAILURE but checker passed:\n#{out}" + elsif !out.include?(fragment) + failures << "#{label}: failed as expected but message missing #{fragment.inspect}:\n#{out}" + end +end + +# A tmp copy of the real fixtures tree, with one fixture file mutated in place. +def with_mutated_fixture(rel) + Dir.mktmpdir("fixture-cov-test") do |dir| + fixtures = File.join(dir, "fixtures") + FileUtils.cp_r(REAL_FIXTURES, fixtures) + path = File.join(fixtures, rel) + data = JSON.parse(read_utf8(path)) + yield data # mutate in place + File.write(path, JSON.generate(data)) + # Point the manifest's fixture lookups at the mutated tree; reuse the real + # manifest (paths are relative to FIXTURE_DIR). + out, status = run_checker(manifest: File.join(fixtures, "manifest.yaml"), fixtures: fixtures) + [out, status] + end +end + +# A tmp manifest (built from the real one, then mutated) against real fixtures. +def with_mutated_manifest + manifest = YAML.safe_load(read_utf8(REAL_MANIFEST)) + yield manifest + Dir.mktmpdir("fixture-cov-test") do |dir| + path = File.join(dir, "manifest.yaml") + File.write(path, YAML.dump(manifest)) + run_checker(manifest: path) + end +end + +# --- Positive control ---------------------------------------------------------- + +out, status = run_checker(manifest: REAL_MANIFEST) +expect_pass(failures, "real manifest passes", out, status) + +# --- Fixture-content mutations ------------------------------------------------- + +out, status = with_mutated_fixture("comments/get.json") { |d| d["id"] = "not-a-number" } +expect_fail(failures, "required field wrong type", out, status, "expected integer, got string") + +out, status = with_mutated_fixture("comments/get.json") { |d| d["id"] = nil } +expect_fail(failures, "required field null", out, status, "required field is null") + +out, status = with_mutated_fixture("comments/get.json") { |d| d["id"] = 1.5 } +expect_fail(failures, "non-integral float for integer", out, status, "expected integer, got number") + +out, status = with_mutated_fixture("comments/get.json") { |d| d["content_attachments"] = [nil] } +expect_fail(failures, "null array element", out, status, "null array element") + +# --- Manifest mutations -------------------------------------------------------- + +out, status = with_mutated_manifest do |m| + t = m["targets"].find { |e| e["id"] == "richtext-comment-0" } + t["pointer"] = "/content_attachments/01" +end +expect_fail(failures, "RFC 6901 leading-zero pointer", out, status, "does not resolve") + +out, status = with_mutated_manifest do |m| + t = m["targets"].find { |e| e["id"] == "richtext-comment-0" } + t["pointer"] = "/content_attachments/~2" +end +expect_fail(failures, "RFC 6901 bad escape", out, status, "does not resolve") + +out, status = with_mutated_manifest do |m| + m["covered_schemas"].delete("Todo") + m["targets"].reject! { |e| e["id"] == "todo-get" } +end +expect_fail(failures, "dropped emitter (self-erasing)", out, status, "neither in covered_schemas") + +out, status = with_mutated_manifest do |m| + t = m["targets"].find { |e| e["id"] == "todo-get" } + t.delete("schema") + t["operation"] = "GetTodo" + t["pointer"] = "/id" +end +expect_fail(failures, "operation entry with non-root pointer", out, status, "non-root") + +# --- Synthetic emitter-discovery cases ----------------------------------------- +# +# Exercise indirect rich-text-emitter shapes against a crafted OpenAPI (no +# targets, so no fixtures are read): the completeness check must still discover +# emitters declared through component-level composition and aliased array items, +# and must traverse reference/composition cycles without hanging. + +SYNTHETIC_OPENAPI = { + "components" => { "schemas" => { + "RichTextAttachment" => { "type" => "object", "properties" => { "id" => { "type" => "integer" } } }, + # A whole-component alias of RichTextAttachment (an aliased item target). + "RTAAlias" => { "$ref" => "#/components/schemas/RichTextAttachment" }, + "Base" => { "type" => "object", "properties" => { "title" => { "type" => "string" } } }, + # (1) Emitter whose companion array arrives via COMPONENT-LEVEL allOf. + "AllOfEmitter" => { "allOf" => [ + { "$ref" => "#/components/schemas/Base" }, + { "type" => "object", + "properties" => { "content_attachments" => { "type" => "array", + "items" => { "$ref" => "#/components/schemas/RichTextAttachment" } } } }, + ] }, + # (2) Emitter whose array ITEMS reference RichTextAttachment through an alias. + "AliasedItemEmitter" => { "type" => "object", + "properties" => { "description_attachments" => { "type" => "array", + "items" => { "$ref" => "#/components/schemas/RTAAlias" } } } }, + # (2b) Emitter declared as a $ref WITH sibling local properties (OpenAPI 3.1) + # — the local content_attachments must be seen despite the $ref. + "SiblingEmitter" => { "$ref" => "#/components/schemas/Base", + "properties" => { "content_attachments" => { "type" => "array", + "items" => { "$ref" => "#/components/schemas/RichTextAttachment" } } } }, + # (3a) Component-level composition cycle (no rich text) — must terminate. + "CycleA" => { "allOf" => [{ "$ref" => "#/components/schemas/CycleB" }] }, + "CycleB" => { "allOf" => [{ "$ref" => "#/components/schemas/CycleA" }] }, + # (3b) A self-referential item alias reached from an array — must terminate. + "SelfRefItem" => { "allOf" => [{ "$ref" => "#/components/schemas/SelfRefItem" }] }, + "CycleItemHolder" => { "type" => "object", + "properties" => { "things" => { "type" => "array", + "items" => { "$ref" => "#/components/schemas/SelfRefItem" } } } }, + } }, +}.freeze + +EMPTY_MANIFEST = { "targets" => [], "covered_schemas" => {}, "excluded_schemas" => {} }.freeze + +# A composed COVERED target: ComposedTarget's required fields (base_field, extra) +# live in $ref + allOf branches, and it is a rich-text emitter. An empty-object +# fixture must fail composition-aware required validation. +COMPOSED_OPENAPI = { + "components" => { "schemas" => { + "RichTextAttachment" => { "type" => "object", "properties" => { "id" => { "type" => "integer" } } }, + "Base" => { "type" => "object", "required" => ["base_field"], + "properties" => { "base_field" => { "type" => "string" } } }, + "ComposedTarget" => { "allOf" => [ + { "$ref" => "#/components/schemas/Base" }, + { "type" => "object", "required" => ["extra"], + "properties" => { "extra" => { "type" => "string" }, + "content_attachments" => { "type" => "array", + "items" => { "$ref" => "#/components/schemas/RichTextAttachment" } } } }, + ] }, + } }, +}.freeze + +COMPOSED_MANIFEST = { + "targets" => [{ "id" => "composed", "fixture" => "composed.json", "pointer" => "", "schema" => "ComposedTarget" }], + "covered_schemas" => { "ComposedTarget" => ["composed"] }, + "excluded_schemas" => {}, +}.freeze + +# Writes openapi + manifest (+ optional fixture files) to a tmp tree and runs the +# checker there, killably (so a cycle that hangs discovery is terminated, not +# left blocking the suite). +def run_synthetic(openapi:, manifest:, fixtures: {}) + Dir.mktmpdir("fixture-cov-synthetic") do |dir| + op = File.join(dir, "openapi.json") + mf = File.join(dir, "manifest.yaml") + fx = File.join(dir, "fixtures") + FileUtils.mkdir_p(fx) + File.write(op, JSON.generate(openapi)) + File.write(mf, YAML.dump(manifest)) + fixtures.each do |rel, data| + p = File.join(fx, rel) + FileUtils.mkdir_p(File.dirname(p)) + File.write(p, JSON.generate(data)) + end + run_checker_killable(manifest: mf, openapi: op, fixtures: fx) + end +end + +begin + out, status = run_synthetic(openapi: SYNTHETIC_OPENAPI, manifest: EMPTY_MANIFEST) + # Indirect emitters must all be discovered and flagged as unaccounted. + expect_fail(failures, "component-level allOf emitter discovered", out, status, "`AllOfEmitter`") + expect_fail(failures, "aliased RichTextAttachment item discovered", out, status, "`AliasedItemEmitter`") + expect_fail(failures, "$ref-with-siblings emitter discovered", out, status, "`SiblingEmitter`") + # The cycle components must NOT be misclassified as emitters, and — proven by + # the run returning at all under the killable timeout — discovery terminated. + if out.include?("`CycleA`") || out.include?("`SelfRefItem`") || out.include?("`CycleItemHolder`") + failures << "cycle components should not be flagged as emitters:\n#{out}" + end +rescue Timeout::Error + failures << "emitter discovery did not terminate on a reference/composition cycle (hung)" +end + +# Composition-aware required validation: an empty fixture for a composed covered +# target must fail on the required fields contributed by its $ref/allOf branches. +begin + out, status = run_synthetic(openapi: COMPOSED_OPENAPI, manifest: COMPOSED_MANIFEST, + fixtures: { "composed.json" => {} }) + expect_fail(failures, "composed covered target with empty fixture fails", out, status, "missing required field") + %w[base_field extra].each do |f| + failures << "composed-target test did not report missing `#{f}`:\n#{out}" unless out.include?("`#{f}`") + end +rescue Timeout::Error + failures << "composed-target validation hung" +end + +# Alternative-group (anyOf/oneOf) validation, with the group inherited through +# BOTH allOf and $ref (ComposedAlt -> allOf -> $ref AltBase -> oneOf). A value +# must satisfy at least one branch; an empty object satisfies neither. +ALT_OPENAPI = { + "components" => { "schemas" => { + "AltBase" => { "oneOf" => [ + { "type" => "object", "required" => ["a"], "properties" => { "a" => { "type" => "string" } } }, + { "type" => "object", "required" => ["b"], "properties" => { "b" => { "type" => "string" } } }, + ] }, + "ComposedAlt" => { "allOf" => [{ "$ref" => "#/components/schemas/AltBase" }] }, + } }, +}.freeze + +def alt_manifest + { "targets" => [{ "id" => "alt", "fixture" => "alt.json", "pointer" => "", "schema" => "ComposedAlt" }], + "covered_schemas" => { "ComposedAlt" => ["alt"] }, + "excluded_schemas" => {} } +end + +begin + out, status = run_synthetic(openapi: ALT_OPENAPI, manifest: alt_manifest, fixtures: { "alt.json" => {} }) + expect_fail(failures, "anyOf/oneOf: empty object matches no alternative", out, status, "matches none of") + + out, status = run_synthetic(openapi: ALT_OPENAPI, manifest: alt_manifest, fixtures: { "alt.json" => { "a" => "x" } }) + expect_pass(failures, "anyOf/oneOf: value matching a branch passes", out, status) +rescue Timeout::Error + failures << "alternative-group validation hung" +end + +# allOf nullability is a conjunction: a required field whose schema is +# allOf[nullable-branch, non-nullable-branch] must reject null (one branch +# forbids it), not accept it because a single branch is nullable. +NULLABLE_ALLOF_OPENAPI = { + "components" => { "schemas" => { + "NullAllOfTarget" => { "type" => "object", "required" => ["f"], "properties" => { + "f" => { "allOf" => [ + { "type" => "string", "nullable" => true }, + { "type" => "string" }, + ] }, + } }, + } }, +}.freeze + +def null_allof_manifest + { "targets" => [{ "id" => "nallof", "fixture" => "nallof.json", "pointer" => "", "schema" => "NullAllOfTarget" }], + "covered_schemas" => { "NullAllOfTarget" => ["nallof"] }, + "excluded_schemas" => {} } +end + +begin + out, status = run_synthetic(openapi: NULLABLE_ALLOF_OPENAPI, manifest: null_allof_manifest, + fixtures: { "nallof.json" => { "f" => nil } }) + expect_fail(failures, "allOf nullability is conjunctive (null rejected)", out, status, + "required field is null but its schema is not nullable") + + out, status = run_synthetic(openapi: NULLABLE_ALLOF_OPENAPI, manifest: null_allof_manifest, + fixtures: { "nallof.json" => { "f" => "x" } }) + expect_pass(failures, "allOf field with a concrete value passes", out, status) +rescue Timeout::Error + failures << "allOf-nullability validation hung" +end + +# anyOf/oneOf nullability: a required field typed only by non-null alternatives +# must REJECT null (no branch permits it); an explicitly-nullable alternative +# must ACCEPT it. This is the path {}-fails / matching-object-passes can't reach. +ALT_NULL_OPENAPI = { + "components" => { "schemas" => { + "AltNullReject" => { "type" => "object", "required" => ["f"], "properties" => { + "f" => { "oneOf" => [{ "type" => "string" }, { "type" => "integer" }] }, + } }, + "AltNullAccept" => { "type" => "object", "required" => ["f"], "properties" => { + "f" => { "oneOf" => [{ "type" => "string", "nullable" => true }, { "type" => "integer" }] }, + } }, + } }, +}.freeze + +def alt_null_manifest(schema, id) + { "targets" => [{ "id" => id, "fixture" => "#{id}.json", "pointer" => "", "schema" => schema }], + "covered_schemas" => { schema => [id] }, + "excluded_schemas" => {} } +end + +begin + out, status = run_synthetic(openapi: ALT_NULL_OPENAPI, manifest: alt_null_manifest("AltNullReject", "reject"), + fixtures: { "reject.json" => { "f" => nil } }) + expect_fail(failures, "anyOf/oneOf: non-null alternatives reject required null", out, status, + "required field is null but its schema is not nullable") + + out, status = run_synthetic(openapi: ALT_NULL_OPENAPI, manifest: alt_null_manifest("AltNullAccept", "accept"), + fixtures: { "accept.json" => { "f" => nil } }) + expect_pass(failures, "anyOf/oneOf: an explicitly-nullable alternative accepts null", out, status) +rescue Timeout::Error + failures << "alternative-nullability validation hung" +end + +# allOf type constraints INTERSECT: a property declared in two allOf branches +# with conflicting types must satisfy both. A value matching only one branch +# fails (union would have wrongly accepted it). +INTERSECT_OPENAPI = { + "components" => { "schemas" => { + "IntersectTarget" => { "allOf" => [ + { "type" => "object", "required" => ["f"], "properties" => { "f" => { "type" => "string" } } }, + { "type" => "object", "properties" => { "f" => { "type" => "integer" } } }, + ] }, + } }, +}.freeze + +begin + out, status = run_synthetic( + openapi: INTERSECT_OPENAPI, + manifest: { "targets" => [{ "id" => "isect", "fixture" => "isect.json", "pointer" => "", "schema" => "IntersectTarget" }], + "covered_schemas" => { "IntersectTarget" => ["isect"] }, "excluded_schemas" => {} }, + fixtures: { "isect.json" => { "f" => "x" } }, + ) + expect_fail(failures, "allOf type constraints intersect (conflicting branch)", out, status, "expected integer, got string") +rescue Timeout::Error + failures << "allOf type-intersection validation hung" +end + +# allOf conjoins array `items` from every branch: an element must satisfy all +# item schemas, not just the first branch's. +ITEMS_ALLOF_OPENAPI = { + "components" => { "schemas" => { + "ItemsTarget" => { "allOf" => [ + { "type" => "object", "required" => ["xs"], + "properties" => { "xs" => { "type" => "array", "items" => { "type" => "string" } } } }, + { "type" => "object", + "properties" => { "xs" => { "type" => "array", "items" => { "type" => "integer" } } } }, + ] }, + } }, +}.freeze + +begin + out, status = run_synthetic( + openapi: ITEMS_ALLOF_OPENAPI, + manifest: { "targets" => [{ "id" => "items", "fixture" => "items.json", "pointer" => "", "schema" => "ItemsTarget" }], + "covered_schemas" => { "ItemsTarget" => ["items"] }, "excluded_schemas" => {} }, + fixtures: { "items.json" => { "xs" => ["hello"] } }, + ) + expect_fail(failures, "allOf conjoins array items across branches", out, status, "expected integer, got string") +rescue Timeout::Error + failures << "allOf items-conjoin validation hung" +end + +# OpenAPI 3.1 scalar `type: "null"`: a required field so typed accepts null. +NULL_TYPE_OPENAPI = { + "components" => { "schemas" => { + "NullTypeTarget" => { "type" => "object", "required" => ["n"], "properties" => { "n" => { "type" => "null" } } }, + } }, +}.freeze + +begin + out, status = run_synthetic( + openapi: NULL_TYPE_OPENAPI, + manifest: { "targets" => [{ "id" => "nt", "fixture" => "nt.json", "pointer" => "", "schema" => "NullTypeTarget" }], + "covered_schemas" => { "NullTypeTarget" => ["nt"] }, "excluded_schemas" => {} }, + fixtures: { "nt.json" => { "n" => nil } }, + ) + expect_pass(failures, 'scalar type:"null" required field accepts null', out, status) + + out, status = run_synthetic( + openapi: NULL_TYPE_OPENAPI, + manifest: { "targets" => [{ "id" => "nt", "fixture" => "nt.json", "pointer" => "", "schema" => "NullTypeTarget" }], + "covered_schemas" => { "NullTypeTarget" => ["nt"] }, "excluded_schemas" => {} }, + fixtures: { "nt.json" => { "n" => "not-null" } }, + ) + expect_fail(failures, 'scalar type:"null" rejects a non-null value', out, status, "expected null, got string") +rescue Timeout::Error + failures << "null-type validation hung" +end + +# OpenAPI 3.1 union `type: ["null"]` (array form with only null) must keep its +# null-only constraint too — a non-null value fails, null passes. +NULL_ARRAY_TYPE_OPENAPI = { + "components" => { "schemas" => { + "NullArrTarget" => { "type" => "object", "required" => ["n"], "properties" => { "n" => { "type" => ["null"] } } }, + } }, +}.freeze + +begin + m = { "targets" => [{ "id" => "na", "fixture" => "na.json", "pointer" => "", "schema" => "NullArrTarget" }], + "covered_schemas" => { "NullArrTarget" => ["na"] }, "excluded_schemas" => {} } + out, status = run_synthetic(openapi: NULL_ARRAY_TYPE_OPENAPI, manifest: m, fixtures: { "na.json" => { "n" => nil } }) + expect_pass(failures, 'type:["null"] required field accepts null', out, status) + + out, status = run_synthetic(openapi: NULL_ARRAY_TYPE_OPENAPI, manifest: m, fixtures: { "na.json" => { "n" => "x" } }) + expect_fail(failures, 'type:["null"] rejects a non-null value', out, status, "expected null, got string") +rescue Timeout::Error + failures << "null-array-type validation hung" +end + +# --- Report -------------------------------------------------------------------- + +if failures.empty? + puts "==> Fixture-coverage self-test passed — 1 positive + 8 negative + 17 synthetic cases" + exit 0 +else + warn "Fixture-coverage self-test FAILED:" + failures.each { |f| warn " - #{f}" } + exit 1 +end diff --git a/spec/fixtures/forwards/get.json b/spec/fixtures/forwards/get.json index 60b25627e..d07be72b0 100644 --- a/spec/fixtures/forwards/get.json +++ b/spec/fixtures/forwards/get.json @@ -4,6 +4,9 @@ "created_at": "2024-01-16T14:30:00.000Z", "updated_at": "2024-01-16T14:30:00.000Z", "subject": "Project proposal from client", + "title": "Project proposal from client", + "inherits_status": true, + "visible_to_clients": false, "content": "
Hi team,

Please review the attached proposal for the new project.
", "from": "client@example.com", "type": "Inbox::Forward", diff --git a/spec/fixtures/forwards/reply_get.json b/spec/fixtures/forwards/reply_get.json index cfbdb61e9..ca9fd9cc5 100644 --- a/spec/fixtures/forwards/reply_get.json +++ b/spec/fixtures/forwards/reply_get.json @@ -3,6 +3,9 @@ "status": "active", "created_at": "2024-01-16T15:00:00.000Z", "updated_at": "2024-01-16T15:00:00.000Z", + "title": "Re: Project proposal from client", + "inherits_status": true, + "visible_to_clients": false, "content": "
Thanks for forwarding this. I'll review and get back to you by EOD.
", "type": "Inbox::Forward::Reply", "url": "https://3.basecampapi.com/195539477/buckets/2085958499/inbox_forwards/1069479380/replies/1069479400.json", diff --git a/spec/fixtures/manifest.yaml b/spec/fixtures/manifest.yaml new file mode 100644 index 000000000..e515ee22b --- /dev/null +++ b/spec/fixtures/manifest.yaml @@ -0,0 +1,197 @@ +# Fixture-completeness manifest + coverage invariant. +# +# Purpose: guarantee that every schema in `covered_schemas` has at least one +# live, validated representative among the shared JSON fixtures — so that when a +# NEW required (or behaviorally-changed) field is added to a covered schema, a +# fixture is forced to carry it and the guard catches the omission. A checker +# that validated only whatever entries happen to exist cannot make that +# guarantee; the coverage invariant closes the gap. +# +# Enforced by scripts/check-fixture-coverage.rb (wired into `make check`). The +# checker uses conformance/runner/ruby/schema-walker.rb for operation-entry +# response-schema lookup; the required/type/nullability walk is its own because +# it must be composition-aware ($ref-with-siblings + allOf), which the walker +# is not. +# +# --- Entry semantics ----------------------------------------------------------- +# +# Each `targets` entry is one of: +# +# operation entry — { id, operation: , fixture: } (pointer optional, defaults "") +# Validates the COMPLETE response body of `operation` (response-schema +# lookup prefers 200, then any 2xx, then default) against the fixture named +# by `fixture`. `fixture` is required — the operation supplies the schema, +# the fixture supplies the body to validate against it. +# +# pointer entry — { id, fixture, pointer, schema } +# Validates the object selected by JSON pointer `pointer` within `fixture` +# against the explicit component `schema`. Use this whenever the response +# schema cannot be inferred from an operation, or the discriminator lies: +# * recordings/get.json declares type "Message" on the wire but MUST +# validate as the generic Recording projection, not Message. +# * search/results.json elements declare type "Todo"/"Message"/… but MUST +# validate as SearchResult. +# * a `/content_attachments/0` element is a RichTextAttachment, not the +# array's parent schema. +# `pointer: ""` selects the whole document. `schema` names a component under +# components/schemas. +# +# --- Coverage invariant -------------------------------------------------------- +# +# `covered_schemas` maps each schema this guard promises to keep represented to +# the target id(s) that represent it. The checker enforces, per covered schema: +# * >= 1 non-excluded (active) target maps to it; +# * the concrete-instance rule: at least one mapped active target resolves to +# a concrete, non-null instance whose DECLARED ROOT schema is that component. +# Transitive reachability and empty arrays do NOT count — a Todo fixture with +# "description_attachments": [] reaches RichTextAttachment but instantiates +# none, so it does not cover it; a populated element (pointer +# .../content_attachments/0 declared RichTextAttachment) does. +# +# `excluded_schemas` records schemas deliberately NOT covered this round, each +# with a reason + tracking issue. Exclusions do NOT satisfy coverage. +# +# --- Inventory completeness (non-self-erasing) -------------------------------- +# +# Every component schema that declares an array-of-RichTextAttachment member (the +# #408 rich-text-emitter class this guard exists to protect) MUST appear in +# covered_schemas or excluded_schemas. This is derived from openapi.json, so the +# inventory cannot silently shrink: dropping a rich-text emitter from +# covered_schemas fails the check instead of quietly reducing coverage. +# +# --- Per-target validation ---------------------------------------------------- +# +# Each target's selected instance is validated against its schema for BOTH +# required-field presence AND type/nullability, composition-aware ($ref-with- +# siblings + allOf are merged; anyOf/oneOf groups — including those inherited via +# $ref/allOf — require the value to satisfy at least one branch): a missing +# required field (including one contributed by an allOf branch), a required field +# null against a non-nullable schema, a null array element against a non-nullable +# item schema, a value matching none of an anyOf/oneOf's alternatives, or a value +# whose JSON type contradicts the declared type (e.g. a string where an array is +# required), fails the target. So a fixture that no longer matches the generated +# types is caught, not just one missing a key. +# +# This is a deliberately PARTIAL structural validator (required/types/nullability/ +# arrays/$ref/allOf/at-least-one-alternative) — not a full JSON-Schema +# implementation. See the "Scope" note in scripts/check-fixture-coverage.rb for +# the supported subset and the keywords intentionally out of scope. +# +# A missing fixture, unresolvable pointer, unknown schema, or covered schema +# with no active concrete representative -> the check FAILS. + +targets: + # 15 concrete resource GETs — pointer "" -> component schema. + - id: comment-get + fixture: comments/get.json + pointer: "" + schema: Comment + - id: message-get + fixture: messages/get.json + pointer: "" + schema: Message + - id: document-get + fixture: documents/get.json + pointer: "" + schema: Document + - id: card-get + fixture: cards/get.json + pointer: "" + schema: Card + - id: todolist-get + fixture: todolists/get.json + pointer: "" + schema: Todolist + - id: upload-get + fixture: uploads/get.json + pointer: "" + schema: Upload + - id: schedule-entry-get + fixture: schedules/entry_get.json + pointer: "" + schema: ScheduleEntry + - id: forward-get + fixture: forwards/get.json + pointer: "" + schema: Forward + - id: forward-reply-get + fixture: forwards/reply_get.json + pointer: "" + schema: ForwardReply + - id: client-approval-get + fixture: client_approvals/get.json + pointer: "" + schema: ClientApproval + - id: client-correspondence-get + fixture: client_correspondences/get.json + pointer: "" + schema: ClientCorrespondence + - id: client-reply-get + fixture: client_replies/get.json + pointer: "" + schema: ClientReply + - id: question-answer-get + fixture: checkins/answer.json + pointer: "" + schema: QuestionAnswer + - id: recording-get + # Discriminator lies: wire type is "Message"; validate as generic Recording. + fixture: recordings/get.json + pointer: "" + schema: Recording + - id: todo-get + fixture: todos/get.json + pointer: "" + schema: Todo + + # Polymorphic search projection: element declares its recording type, but + # validates as SearchResult. + - id: search-result-0 + fixture: search/results.json + pointer: "/0" + schema: SearchResult + + # Two concrete RichTextAttachment instances (populated elements, not empty + # arrays) — the only way to cover RichTextAttachment per the concrete-instance + # rule. + - id: richtext-comment-0 + fixture: comments/get.json + pointer: "/content_attachments/0" + schema: RichTextAttachment + - id: richtext-upload-0 + fixture: uploads/get.json + pointer: "/description_attachments/0" + schema: RichTextAttachment + +covered_schemas: + Comment: [comment-get] + Message: [message-get] + Document: [document-get] + Card: [card-get] + Todolist: [todolist-get] + Upload: [upload-get] + ScheduleEntry: [schedule-entry-get] + Forward: [forward-get] + ForwardReply: [forward-reply-get] + ClientApproval: [client-approval-get] + ClientCorrespondence: [client-correspondence-get] + ClientReply: [client-reply-get] + QuestionAnswer: [question-answer-get] + Recording: [recording-get] + Todo: [todo-get] + SearchResult: [search-result-0] + RichTextAttachment: [richtext-comment-0, richtext-upload-0] + +excluded_schemas: + Gauge: + reason: >- + No shared JSON fixture. Gauge is exercised only inline in + go/pkg/basecamp/gauges_test.go, so its optional non-nullable + description_attachments array has no manifest'd representative. + issue: 429 + GaugeNeedle: + reason: >- + No shared JSON fixture (same Gauge inline-only situation). GaugeNeedle + carries a @required description_attachments array not represented in any + shared fixture. + issue: 429 diff --git a/typescript/tests/services/cards.test.ts b/typescript/tests/services/cards.test.ts index 42a8e7c3d..381c7788c 100644 --- a/typescript/tests/services/cards.test.ts +++ b/typescript/tests/services/cards.test.ts @@ -7,18 +7,14 @@ import { server } from "../setup.js"; import { createBasecampClient } from "../../src/client.js"; import { BasecampError } from "../../src/errors.js"; import type { BasecampClient } from "../../src/client.js"; +import cardFixture from "../../../spec/fixtures/cards/get.json"; const BASE_URL = "https://3.basecampapi.com/12345"; -const sampleCard = (id = 1) => ({ - id, - title: "Design mockups", - content: "

Create initial designs

", - description_attachments: [], - due_on: "2024-03-01", - created_at: "2024-01-15T10:00:00Z", - updated_at: "2024-01-15T10:00:00Z", -}); +// Sourced from the shared, coverage-guarded fixture (spec/fixtures/manifest.yaml) +// so this helper cannot drift from the validated Card shape; `id` is overridable +// per call. +const sampleCard = (id = cardFixture.id) => ({ ...cardFixture, id }); describe("CardsService", () => { let client: BasecampClient; @@ -71,7 +67,7 @@ describe("CardsService", () => { const card = await client.cards.get(cardId); expect(card.id).toBe(cardId); - expect(card.title).toBe("Design mockups"); + expect(card.title).toBe(cardFixture.title); }); it("should throw not_found for missing card", async () => { diff --git a/typescript/tests/services/client-visibility.test.ts b/typescript/tests/services/client-visibility.test.ts index 02c6a8bee..750ab19b1 100644 --- a/typescript/tests/services/client-visibility.test.ts +++ b/typescript/tests/services/client-visibility.test.ts @@ -6,17 +6,14 @@ import { http, HttpResponse } from "msw"; import { server } from "../setup.js"; import { createBasecampClient } from "../../src/client.js"; import type { BasecampClient } from "../../src/client.js"; +import recordingFixture from "../../../spec/fixtures/recordings/get.json"; const BASE_URL = "https://3.basecampapi.com/12345"; -const sampleRecording = (id = 1) => ({ - id, - title: "Some recording", - type: "Todo", - visible_to_clients: true, - created_at: "2024-01-15T10:00:00Z", - updated_at: "2024-01-15T10:00:00Z", -}); +// Sourced from the shared, coverage-guarded fixture (spec/fixtures/manifest.yaml) +// so this helper cannot drift from the validated Recording shape; `id` is +// overridable per call. +const sampleRecording = (id = recordingFixture.id) => ({ ...recordingFixture, id }); describe("ClientVisibilityService", () => { let client: BasecampClient; diff --git a/typescript/tests/services/comments.test.ts b/typescript/tests/services/comments.test.ts index fef0a261a..6b7c13672 100644 --- a/typescript/tests/services/comments.test.ts +++ b/typescript/tests/services/comments.test.ts @@ -7,17 +7,14 @@ import { server } from "../setup.js"; import { createBasecampClient } from "../../src/client.js"; import { BasecampError } from "../../src/errors.js"; import type { BasecampClient } from "../../src/client.js"; +import commentFixture from "../../../spec/fixtures/comments/get.json"; const BASE_URL = "https://3.basecampapi.com/12345"; -const sampleComment = (id = 1) => ({ - id, - content: "

Great work!

", - content_attachments: [], - created_at: "2024-01-15T10:00:00Z", - updated_at: "2024-01-15T10:00:00Z", - creator: { id: 100, name: "Jane Doe" }, -}); +// Sourced from the shared, coverage-guarded fixture (spec/fixtures/manifest.yaml) +// so this helper cannot drift from the validated Comment shape; `id` is +// overridable per call. +const sampleComment = (id = commentFixture.id) => ({ ...commentFixture, id }); describe("CommentsService", () => { let client: BasecampClient; @@ -42,7 +39,7 @@ describe("CommentsService", () => { const comment = await client.comments.get(commentId); expect(comment.id).toBe(commentId); - expect(comment.content).toBe("

Great work!

"); + expect(comment.content).toBe(commentFixture.content); }); it("preserves float-spelled and null attachment dimensions at runtime", async () => { diff --git a/typescript/tests/services/messages.test.ts b/typescript/tests/services/messages.test.ts index a10801c81..f007b761b 100644 --- a/typescript/tests/services/messages.test.ts +++ b/typescript/tests/services/messages.test.ts @@ -7,19 +7,14 @@ import { server } from "../setup.js"; import { createBasecampClient } from "../../src/client.js"; import { BasecampError } from "../../src/errors.js"; import type { BasecampClient } from "../../src/client.js"; +import messageFixture from "../../../spec/fixtures/messages/get.json"; const BASE_URL = "https://3.basecampapi.com/12345"; -const sampleMessage = (id = 1) => ({ - id, - subject: "Weekly Update", - content: "

Here is the update

", - content_attachments: [], - status: "active", - created_at: "2024-01-15T10:00:00Z", - updated_at: "2024-01-15T10:00:00Z", - creator: { id: 100, name: "Jane Doe" }, -}); +// Sourced from the shared, coverage-guarded fixture (spec/fixtures/manifest.yaml) +// so this helper cannot drift from the validated Message shape; `id` is +// overridable per call. +const sampleMessage = (id = messageFixture.id) => ({ ...messageFixture, id }); describe("MessagesService", () => { let client: BasecampClient; @@ -88,7 +83,7 @@ describe("MessagesService", () => { const message = await client.messages.get(messageId); expect(message.id).toBe(messageId); - expect(message.subject).toBe("Weekly Update"); + expect(message.subject).toBe(messageFixture.subject); }); it("should throw not_found for missing message", async () => { diff --git a/typescript/tests/services/todolists.test.ts b/typescript/tests/services/todolists.test.ts index 9967f8d8d..636a8f0fb 100644 --- a/typescript/tests/services/todolists.test.ts +++ b/typescript/tests/services/todolists.test.ts @@ -7,19 +7,14 @@ import { server } from "../setup.js"; import { createBasecampClient } from "../../src/client.js"; import { BasecampError } from "../../src/errors.js"; import type { BasecampClient } from "../../src/client.js"; +import todolistFixture from "../../../spec/fixtures/todolists/get.json"; const BASE_URL = "https://3.basecampapi.com/12345"; -const sampleTodolist = (id = 1) => ({ - id, - name: "Launch list", - description: "

Things to do before launch

", - description_attachments: [], - completed: false, - completed_ratio: "0/5", - created_at: "2024-01-15T10:00:00Z", - updated_at: "2024-01-15T10:00:00Z", -}); +// Sourced from the shared, coverage-guarded fixture (spec/fixtures/manifest.yaml) +// so this helper cannot drift from the validated Todolist shape; `id` is +// overridable per call. +const sampleTodolist = (id = todolistFixture.id) => ({ ...todolistFixture, id }); describe("TodolistsService", () => { let client: BasecampClient; @@ -44,7 +39,7 @@ describe("TodolistsService", () => { const todolist = await client.todolists.get(id); expect(todolist.id).toBe(id); - expect(todolist.name).toBe("Launch list"); + expect(todolist.name).toBe(todolistFixture.name); }); it("should throw not_found for missing todolist", async () => { diff --git a/typescript/vitest.config.ts b/typescript/vitest.config.ts index d32cdb3e1..12f128d11 100644 --- a/typescript/vitest.config.ts +++ b/typescript/vitest.config.ts @@ -1,6 +1,20 @@ import { defineConfig } from "vitest/config"; +import { fileURLToPath } from "node:url"; + +// Repo root (parent of typescript/). Service tests source their stub bodies from +// the shared, coverage-guarded fixtures under /spec/fixtures via JSON +// imports; pin the fs allow-list to the repo root so those out-of-package +// imports resolve regardless of Vite's workspace-root auto-detection (which a +// nested package.json or a strict downstream environment could otherwise +// narrow). +const repoRoot = fileURLToPath(new URL("..", import.meta.url)); export default defineConfig({ + server: { + fs: { + allow: [repoRoot], + }, + }, test: { globals: true, environment: "node",