diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 4a87029..b8f4f2e 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -5,15 +5,15 @@ jobs: name: Test haskell codes runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 - - uses: haskell/actions/setup@v1 + - uses: actions/checkout@v4 + - uses: haskell-actions/setup@v2 id: haskell-setup with: ghc-version: "8.8.3" enable-stack: true stack-version: "2.5.1" - name: Restore caches - uses: actions/cache@v2 + uses: actions/cache@v4 with: path: | ${{steps.haskell-setup.outputs.stack-path}} @@ -22,19 +22,18 @@ jobs: key: ${{ runner.os }}-${{ hashFiles('**/package.yaml') }}-${{ hashFiles('**/onix.cabal') }}-${{ hashFiles('**/stack.yaml.lock') }} restore-keys: | ${{ runner.os }}- - - run: ls -lah ${{steps.haskell-setup.outputs.stack-path}} - run: make test e2e: name: Test e2e runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 - name: Setup Node.js environment - uses: actions/setup-node@v2.1.4 + uses: actions/setup-node@v4 with: node-version: 12.x - name: Restore caches - uses: actions/cache@v2 + uses: actions/cache@v4 with: path: | ~/.npm diff --git a/Makefile b/Makefile index b08d1dd..3c32245 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,9 @@ TS_FILES := $(shell find ./ -type f -name '*.ts' | grep -v 'node_modules') HS_FILES := $(shell find ./ -type f -name '*.hs' | grep -v '.stack-work') BZL := npx bazelisk -BZL_BIN := $(shell npx bazel info bazel-bin) +# Deferred on purpose: `:=` would run bazel on every make invocation, including +# `make test`, which runs in a job with no node_modules and no need for bazel. +BZL_BIN = $(shell $(BZL) info bazel-bin) generated/go/%: build stack exec onix-exe -- --schemaVersion $(@F) --language go @@ -12,8 +14,14 @@ generated/ts/%: build debug: build stack exec --trace -- onix-exe +RTS -xc --RTS --schemaVersion v3 --language go +# The fixtures under fixtures/ are self-contained, so the tests deliberately do +# not depend on the `schema` target: `make schema` downloads the EDItEUR +# archives over the network, and requiring it here made the test suite +# unrunnable whenever editeur.org was unreachable. Keep fixtures from including +# anything outside fixtures/, or this dependency comes back. +# See docs/adr/0002-decouple-unit-tests-from-the-vendored-schema.md .PHONY: test -test: schema +test: stack test --trace --fast .stack-work: $(HS_FILES) package.yaml stack.yaml @@ -21,10 +29,6 @@ test: schema build: schema .stack-work -json: fixtures/20201200.json -fixtures/20201200.json: run - go run github.com/kogai/onix-codegen/go/helper - WORKSPACE: go.mod $(BZL) run //:gazelle -- update-repos -from_file=go.mod diff --git a/WORKSPACE b/WORKSPACE index 54498ad..735a6d5 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -14,22 +14,6 @@ http_archive( url = "https://www.editeur.org/files/ONIX%203/ONIX_BookProduct_XSD_schema+codes_Issue_52.zip", ) -http_archive( - name = "build_bazel_rules_nodejs", - sha256 = "dd4dc46066e2ce034cba0c81aa3e862b27e8e8d95871f567359f7a534cccb666", - urls = ["https://github.com/bazelbuild/rules_nodejs/releases/download/3.1.0/rules_nodejs-3.1.0.tar.gz"], -) - -# The npm_install rule runs yarn anytime the package.json or package-lock.json file changes. -# It also extracts any Bazel rules distributed in an npm package. -load("@build_bazel_rules_nodejs//:index.bzl", "npm_install") - -npm_install( - name = "npm", - package_json = "//:package.json", - package_lock_json = "//:package-lock.json", -) - http_archive( name = "io_bazel_rules_go", sha256 = "207fad3e6689135c5d8713e5a17ba9d1290238f47b9ba545b63d9303406209c6", diff --git a/docs/adr/0000-template.md b/docs/adr/0000-template.md new file mode 100644 index 0000000..f32c01b --- /dev/null +++ b/docs/adr/0000-template.md @@ -0,0 +1,26 @@ +# ADR-0000: タイトル + +- **ステータス**: Proposed | Accepted | Superseded by ADR-XXXX +- **日付**: YYYY-MM-DD + +## 背景 + +どういう状況で、何が問題になっているか。観測された事実 (エラーメッセージ、計測値、 +外部サービスの挙動) を具体的に書く。 + +## 決定 + +何を決めたか。 + +## 理由 + +なぜその選択肢を選んだか。 + +## 検討した他の選択肢 + +- **案 A**: 内容と、採らなかった理由 +- **案 B**: 内容と、採らなかった理由 + +## 結果 + +この決定によって何が変わるか。新しく発生する制約や、将来見直すべき条件も書く。 diff --git a/docs/adr/0001-record-architecture-decisions.md b/docs/adr/0001-record-architecture-decisions.md new file mode 100644 index 0000000..043bc9c --- /dev/null +++ b/docs/adr/0001-record-architecture-decisions.md @@ -0,0 +1,52 @@ +# ADR-0001: 設計判断を ADR として記録する + +- **ステータス**: Accepted +- **日付**: 2026-09-13 + +## 背景 + +onix-codegen には、コードを読むだけでは意図が復元できない判断がいくつもある。例えば次のようなもの。 + +- スキーマの版を `v2` / `v3` というディレクトリで分け、既存の出力を置き換えない +- 言語固有の知識をテンプレートに閉じ込め、`src/` を言語非依存に保つ +- 対応していない XSD 構造は暗黙に無視せず `unimplemented` / `unreachable` で落とす + +これらはいずれも「後方互換性を保ちながら ONIX 仕様に追従する」「サポート言語を増やしやすくする」という +このリポジトリの価値 (AGENTS.md) から導かれているが、根拠がコミットログに散っているため、 +後から来た人が「なぜこうなっているのか」を再構成しづらい。結果として、良かれと思った変更が +その判断を静かに壊すことがある。 + +外部依存の扱いも同じ問題を抱えている。EDItEUR の配布物、Bazel、Stackage のスナップショットは +どれも外部の都合で壊れるが、「なぜこのバージョンに固定しているのか」が残っていないと、 +更新のたびに同じ調査をやり直すことになる。 + +## 決定 + +設計判断を `docs/adr/` 配下の ADR として記録する。フォーマットは MADR を簡略化したもので、 +`0000-template.md` を雛形とする。連番は一度振ったら変えず、判断が変わったときは既存の ADR を +書き換えるのではなく、新しい ADR を書いて古いものを `Superseded` にする。 + +AGENTS.md には、どういう判断を ADR にすべきかの基準を書き、両者を相互に参照させる。 + +## 理由 + +ADR は「決定そのもの」ではなく「決定に至った制約」を残せる点が、コメントやコミットメッセージより優れている。 +このリポジトリで重要なのは、まさにその制約 (EDItEUR の配布形態、生成物の互換性、テンプレートの責務分担) が +時間とともに変わることなので、当時の前提ごと記録に残せる形式が要る。 + +Markdown をリポジトリ内に置くのは、スキーマや生成物と同じコミットで変更履歴が追えるため。 +外部の Wiki やイシューに書くと、コードとの対応が切れる。 + +## 検討した他の選択肢 + +- **AGENTS.md / README.md に全部書く**: 判断の履歴が積み上がると読みものとして破綻する。 + AGENTS.md は「今どうすべきか」を書く場所として保ち、「なぜそうなったか」は ADR に分ける。 +- **GitHub Issues / Discussions に残す**: 検索性はあるが、リポジトリを clone しただけでは読めず、 + コードとの対応も切れる。オフラインで完結しない。 +- **記録しない (現状維持)**: 調査のやり直しと、意図しない互換性破壊が続く。 + +## 結果 + +- 新しい設計判断、および後から言語化された既知の判断は ADR に書く。 +- ADR を追加したら `docs/adr/README.md` の一覧も更新する。 +- ADR はレビュー対象になる。判断に異論があれば、実装ではなく ADR に対してコメントできる。 diff --git a/docs/adr/0002-decouple-unit-tests-from-the-vendored-schema.md b/docs/adr/0002-decouple-unit-tests-from-the-vendored-schema.md new file mode 100644 index 0000000..2de7097 --- /dev/null +++ b/docs/adr/0002-decouple-unit-tests-from-the-vendored-schema.md @@ -0,0 +1,106 @@ +# ADR-0002: ユニットテストを取得済みスキーマから切り離す + +- **ステータス**: Accepted +- **日付**: 2026-09-13 + +## 背景 + +Makefile のテストターゲットは `schema` に依存していた。 + +```make +.PHONY: test +test: schema + stack test --trace --fast +``` + +`schema` ターゲットは Bazel の `http_archive` 経由で EDItEUR の zip をダウンロードし、 +`schema/v2` / `schema/v3` に展開する。つまり `make test` は毎回 editeur.org への +ネットワークアクセスを要求していた。 + +2026-09-13 時点で、この取得が機能しなくなった。 + +``` +WARNING: Download from https://www.editeur.org/files/ONIX%202.1/ONIX_for_Books_Release2-1_rev03_schema+codes_Issue_36.zip + failed: UnrecoverableHttpException GET returned 202 Accepted +ERROR: An error occurred during the fetch of repository 'org_editeur_v2' +make: *** [Makefile:35: schema/v2] Error 1 +``` + +その結果、`Test haskell codes` ジョブはテストを 1 件も実行しないまま失敗する。 +取得できない理由そのものは ADR-0003 で扱う。 + +### この依存は本物だった + +当初、この依存は単なる過剰指定だと考えた。テストコードが直接読むのは +`fixtures/test_*.xsd` だけで、`./schema` という文字列は `src/Lib.hs` の `schemaRoot` +にしか現れないからである。しかしこれは誤りだった。 + +`fixtures/test_mixed_html.xsd` が、EDItEUR の配布物を直接 include していた。 + +```xml + +``` + +`Xsd.getSchema` は include を再帰的にたどり (`src/Xsd.hs` の `go` / `goInclude`)、 +相対パスを `combineURIs` でローカルパスに解決して `Text.XML.readFile` で読む。 +ファイルが無ければ IOException で落ちる。このフィクスチャは +`test/TestMixed.hs` と `test/TestModel.hs` の両方から読まれており、`schema/` は +`.gitignore` されているので、clean checkout では必ず存在しない。 + +つまり `test: schema` は正しい依存だった。git の履歴もそれを裏づけている。 +`test: schema` を追加したコミット (9352123) は、リポジトリに置かれていた +`2_1_rev03_schema/` を削除し、このフィクスチャの include を `../schema/v2/` に +向け直した、まさにそのコミットである。 + +## 決定 + +依存を消すのではなく、**依存の対象をリポジトリ内に移す**。 + +1. `fixtures/test_mixed_html_xhtml_subset.xsd` を追加する。`test_mixed_html.xsd` が + `ref` している 40 個の要素名だけを宣言した、ONIX_XHTML_Subset.xsd の代替物である。 +2. `test_mixed_html.xsd` の include をそちらに向ける。 +3. そのうえで `test` ターゲットから `schema` 依存を外す。 + +`build` ターゲットの `schema` 依存は残す。実際にコードを生成するにはスキーマの実体が +要るため、こちらは今も本物の依存である。 + +## 理由 + +代替物は、テストが実際に検証している性質を保つように書いた。 +`test/TestModel.hs` は、このフィクスチャを読んだうえで `Model.collectElements` が +空になることを表明している。`collectElements` は `complexMixed = False` の要素だけを +拾うフィルタなので、この表明の意味は「XHTML の要素がモデルに漏れてこない」ことである。 +XHTML の内容要素はいずれも mixed content なので、代替物でも全要素を +`` として宣言した。要素名の集合が実物と一致していること、 +フィクスチャ側の `ref` 40 個すべてに宣言が対応することは機械的に確認した。 + +代替物で足りるのは、テストがこのファイルから必要としているのが**要素の宣言の存在と +mixed であること**だけだからである。ONIX_XHTML_Subset.xsd の完全な内容 (属性、 +コンテンツモデルの詳細) は、ここで検証されている性質に寄与していない。 + +そのうえで、テストを外部サービスの可用性から切り離す価値は大きい。パーサのバグを +直したいときに editeur.org の状態に左右されるのは、依存の向きとして誤っている。 + +なお、この変更は 202 の問題そのものを解決しない。コード生成と、生成物を最新スキーマへ +追従させる作業は依然としてダウンロードを必要とする (ADR-0003)。 + +## 検討した他の選択肢 + +- **`test: schema` を残す**: 事実としては正しい依存なので、これは筋が通っている。 + ただし、たった 1 ファイルの XSD のためにテスト全体を外部サービスに縛り続けることになる。 +- **ONIX_XHTML_Subset.xsd をそのまま `fixtures/` に取り込む**: 代替物より忠実だが、 + EDItEUR の配布物の再配布にあたる。ライセンスの判断が要るため、テストを通すためだけに + 踏み込むべきではない (ADR-0003 と同じ理由)。代替物は自前で書いたものなのでこの問題がない。 +- **include を単に削除する**: `Annotation` 以外に要素が無くなるので `collectElements` は + 空になり、表明は通ってしまう。だが「XHTML 要素が漏れてこない」ことを何も検証しなくなり、 + テストが意味を失う。**採らない。** + +## 結果 + +- `make test` はネットワークなしで実行できる。CI の Haskell ジョブは editeur.org に依存しない。 +- `fixtures/test_mixed_html_xhtml_subset.xsd` は実物の代替物である。実物側の構造が変わって + テストの前提が動くことはあり得るので、スキーマの版を上げるときはこのファイルも見直す。 +- コード生成 (`make build`、`make generated/...`) は引き続きダウンロードを必要とする。 +- 新しいテストを書くときは `fixtures/` に最小の XSD を足す、という既存の慣習が + そのまま「テストを外部依存から切り離す」ことにもなる。この性質は維持する。 + フィクスチャから `fixtures/` の外を include しないこと。 diff --git a/docs/adr/0004-drop-rules-nodejs-from-the-e2e-test.md b/docs/adr/0004-drop-rules-nodejs-from-the-e2e-test.md new file mode 100644 index 0000000..e6f3d9c --- /dev/null +++ b/docs/adr/0004-drop-rules-nodejs-from-the-e2e-test.md @@ -0,0 +1,80 @@ +# ADR-0004: e2e スナップショット比較から rules_nodejs を外す + +- **ステータス**: Accepted +- **日付**: 2026-09-13 + +## 背景 + +`build_bazel_rules_nodejs` 3.1.0 は WORKSPACE の依存として宣言されていたが、 +実際に使われていたのは次の 2 箇所だけだった。 + +``` +WORKSPACE:18 name = "build_bazel_rules_nodejs", +WORKSPACE:25 load("@build_bazel_rules_nodejs//:index.bzl", "npm_install") +e2e/go/BUILD.bazel:2 load("@build_bazel_rules_nodejs//:index.bzl", "generated_file_test") +``` + +このうち `npm_install` が定義する `@npm` リポジトリは、**どの BUILD ターゲットからも +参照されていない**。`//e2e/go:snapshot_test` は Go のバイナリと JSON ファイルしか使わない。 +つまり実質的な用途は `generated_file_test` ただ 1 つだった。 + +この依存はライブラリ更新の妨げにもなっていた。上げ先を順に見ると、行き止まりになっている。 + +- **4.7.0 / 5.8.5**: `generated_file_test` は存在する。ただし 5.8.5 の + `SUPPORTED_BAZEL_VERSIONS` は `["4.2.2", "5.0.0"]` で、現行の `.bazelversion` (3.7.0) を + 含まない。つまり Bazel 側も同時に上げないと使えない。 +- **6.x**: rules_nodejs が大きく整理され、ルート `index.bzl` ごと無くなった。 + `generated_file_test` は現行版には存在しない。 + +したがって「rules_nodejs だけ上げる」は成立せず、最新まで上げるならどのみち比較の仕組みを +書き換えることになる。書き換えたうえで依存だけが残る。 + +なお、Bazel 自体の起動に使っている `npx bazelisk` は npm の devDependency であって +rules_nodejs とは無関係なので、この判断の影響を受けない。 + +## 決定 + +`generated_file_test` を、外部ルールに依存しない `sh_test` (`e2e/go/snapshot_test.sh`) に +置き換え、`build_bazel_rules_nodejs` を WORKSPACE から削除する。 + +スクリプトは生成された JSON とコミット済みスナップショットを `diff -u` で比較し、 +差分があれば差分そのものを表示して失敗する。 + +## 理由 + +比較の中身は「2 つのファイルが同一か」でしかない。そのために外部リポジトリを 1 つ +丸ごと取得するのは釣り合っていない。`diff` は Bazel のバージョンにも依存しないので、 +今後 Bazel を上げるときにこの部分が障害にならない。 + +`bazel_skylib` の `diff_test` に置き換える案もあったが、それは依存を別の依存に +入れ替えるだけで、しかも skylib のどのバージョンが Bazel 3.7.0 と組み合わせられるかを +この環境では検証できない (`releases.bazel.build` に到達できない)。検証できない選択肢を +2 つ抱えるより、依存を減らして 1 つに絞るほうがよい。 + +失敗時の出力はむしろ改善する。`generated_file_test` は不一致の事実を報告するだけだったが、 +`diff -u` は何がどう違うかを出す。スナップショットの更新手順もスクリプトの +コメントに書いた。 + +## 検討した他の選択肢 + +- **rules_nodejs を 5.8.5 に上げる**: `generated_file_test` はまだあるが、 + 対応 Bazel が 4.2.2 / 5.0.0 なので Bazel も同時に上げる必要があり、変更が連鎖する。 +- **rules_nodejs を 6.x に上げる**: `generated_file_test` が無いので、 + どのみち比較の仕組みを書き換えることになる。書き換えたうえで依存が残るだけ損。 +- **`bazel_skylib` の `diff_test` を使う**: 標準的で堅い。ただし上記のとおり、 + 依存を入れ替えるだけで減らず、バージョン組み合わせを検証できない。 +- **`npm_install` だけ残す**: 参照するターゲットが無いので、残す理由が無い。 + +## 結果 + +- WORKSPACE の外部依存が 1 つ減り、Bazel のバージョンを上げるときの制約も 1 つ減る。 +- `package.json` / `package-lock.json` は Bazel のビルドグラフから完全に外れた。 + これらが影響するのは `npx bazelisk` の取得だけになる。 +- スナップショットの更新は手作業になった (`bazel build //e2e/go:snapshot` の出力を + `fixtures/20201200.json` にコピー)。手順は `e2e/go/snapshot_test.sh` の冒頭にある。 + あわせて Makefile の `json` ターゲットを削除した。存在しない `run` ターゲットに依存し、 + 実在しない import path (`.../go/helper`、実体は `e2e/go`) を叩く二重に壊れた状態で、 + 更新手段として機能していなかった。 +- 旧 `generated_file_test` は `src` と `generated` の指定が逆で、付随する + `:snapshot_test.update` はスナップショットではなく生成物側を書き換えようとしていた。 + 比較そのものは対称なので検証は機能していたが、更新経路は元から使えなかった。 diff --git a/docs/adr/README.md b/docs/adr/README.md new file mode 100644 index 0000000..7bffd6f --- /dev/null +++ b/docs/adr/README.md @@ -0,0 +1,27 @@ +# Architecture Decision Records (ADR) + +このディレクトリには、onix-codegen の設計判断を記録する ADR を置く。 + +## 一覧 + +| # | タイトル | ステータス | +| ------------------------------------------------------------- | ---------------------------------------------- | ---------- | +| [0001](0001-record-architecture-decisions.md) | 設計判断を ADR として記録する | Accepted | +| [0002](0002-decouple-unit-tests-from-the-vendored-schema.md) | ユニットテストを取得済みスキーマから切り離す | Accepted | +| [0004](0004-drop-rules-nodejs-from-the-e2e-test.md) | e2e から rules_nodejs を外す | Accepted | + +## 書き方 + +`0000-template.md` をコピーして、次の連番を付ける。番号は一度振ったら変えない。 + +ADR は「その時点で、どういう制約のもとに、なぜそう決めたか」を残すもの。 +後から判断が変わったら、既存の ADR を書き換えるのではなく、新しい ADR を書いて +古いものの Status を `Superseded by ADR-XXXX` に更新する。決定の履歴が消えないことが重要。 + +## どういう判断を ADR にするか + +このリポジトリの価値 (AGENTS.md 参照) に照らして、次のいずれかに影響するもの。 + +- **後方互換性**: 生成コードの公開 API に影響する判断、スキーマの版の扱い方 +- **サポート言語の増やしやすさ**: 中間表現とテンプレートの責務分担、言語追加の手順に影響する判断 +- **ビルドと CI の前提**: 外部依存 (EDItEUR の配布物、ツールチェーンのバージョン) の扱い方 diff --git a/e2e/go/BUILD.bazel b/e2e/go/BUILD.bazel index c01fbc7..93d2457 100644 --- a/e2e/go/BUILD.bazel +++ b/e2e/go/BUILD.bazel @@ -1,5 +1,4 @@ load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library") -load("@build_bazel_rules_nodejs//:index.bzl", "generated_file_test") filegroup( name = "fixtures", @@ -33,8 +32,16 @@ genrule( tools = ["helper"], ) -generated_file_test( +sh_test( name = "snapshot_test", - src = "snapshot", - generated = "//:fixtures/20201200.json", + size = "small", + srcs = ["snapshot_test.sh"], + args = [ + "$(location :out.json)", + "$(location //:fixtures/20201200.json)", + ], + data = [ + ":out.json", + "//:fixtures/20201200.json", + ], ) diff --git a/e2e/go/snapshot_test.sh b/e2e/go/snapshot_test.sh new file mode 100755 index 0000000..2b13606 --- /dev/null +++ b/e2e/go/snapshot_test.sh @@ -0,0 +1,22 @@ +#!/usr/bin/env bash +# Compares the JSON produced by reading fixtures/20201200.onix through the +# generated Go client against the committed snapshot. A difference means the +# generated client's runtime behaviour changed. +# +# To accept a new snapshot: +# npx bazelisk build //e2e/go:snapshot +# cp bazel-bin/e2e/go/out.json fixtures/20201200.json +set -o errexit +set -o nounset +set -o pipefail + +actual="$1" +expected="$2" + +if ! diff -u "$expected" "$actual"; then + echo "" >&2 + echo "The generated Go client no longer reproduces fixtures/20201200.json." >&2 + echo "If the change is intended, refresh the snapshot as described in" >&2 + echo "e2e/go/snapshot_test.sh." >&2 + exit 1 +fi diff --git a/fixtures/test_mixed_html.xsd b/fixtures/test_mixed_html.xsd index 110ce0e..642cdf2 100644 --- a/fixtures/test_mixed_html.xsd +++ b/fixtures/test_mixed_html.xsd @@ -1,6 +1,6 @@ - + diff --git a/fixtures/test_mixed_html_xhtml_subset.xsd b/fixtures/test_mixed_html_xhtml_subset.xsd new file mode 100644 index 0000000..5be1621 --- /dev/null +++ b/fixtures/test_mixed_html_xhtml_subset.xsd @@ -0,0 +1,53 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +