Skip to content

Add per-case test timeout via oj --tle#130

Merged
toshi0806 merged 5 commits into
mainfrom
add-test-timeout
Jun 23, 2026
Merged

Add per-case test timeout via oj --tle#130
toshi0806 merged 5 commits into
mainfrom
add-test-timeout

Conversation

@toshi0806

Copy link
Copy Markdown
Member

概要

am t / am tf でテスト実行時にタイムアウト処理を追加しました。無限ループや極端に遅いコードを書いてもテストが返ってこずハングする問題を解消します。

変更内容

oj ネイティブの --tle オプションを test / tf の make ターゲットに渡すようにしました。ammakeoj の実行経路のうち、実際にコードを起動・計測している oj にタイムアウトを任せるのが素直なため、makefile 側で対応しています(am 自体は変更不要)。

  • lib/.support/makefile: OJ_TLE 変数を追加し OJ_TFLAGS--tle $(OJ_TLE) を付与。t / tf の両方に効きます。
  • CLAUDE.md: 仕様と使い方を追記。

設計のポイント

  • デフォルト 4 秒: AtCoder の標準制限は 2 秒ですが、Dev Container は judge より遅い場合があるため、誤 TLE を避けて余裕を持たせています。
  • 問題ごとに上書き可能: ?= と環境変数継承により OJ_TLE=2 am t .java で変更できます。
  • 全コンテストに即反映: 各問題の makefile はこの中央ファイルへのシンボリックリンクのため、既存・新規問わず反映されます。

動作確認

検証 結果
通常テスト(abc459/a, Java) --tle 4 付きで 3 ケース全 AC ✅
無限ループ + OJ_TLE=2 約 2 秒で打ち切り、[FAILURE] TLE 表示、ハングなし ✅

Pass --tle to oj in the test/tf make targets so that infinite loops or
overly slow code are killed and reported as TLE instead of hanging.

- Default OJ_TLE=4s: AtCoder's limit is 2s, but the dev container can be
  slower than the judge, so allow headroom to avoid false TLE.
- Overridable per task: OJ_TLE=2 am t .java
- Document the behavior and override in CLAUDE.md
Copilot AI review requested due to automatic review settings June 23, 2026 01:26

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@toshi0806 toshi0806 closed this Jun 23, 2026
@toshi0806 toshi0806 reopened this Jun 23, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

全体的にシンプルで意図が明確な変更です。設計上の重大な問題は見当たりませんが、1点だけ注意すべき点があります。

Comment thread lib/.support/makefile
# dev container can be slower than the judge, so allow extra headroom to
# avoid false TLE. Override per task with e.g. `OJ_TLE=2 am t .java`.
OJ_TLE ?= 4
OJ_TFLAGS = -c '$(RUN_TEST)' --ignore-spaces-and-newline --tle $(OJ_TLE)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ [LOW] OJ_TLE?= で定義していますが、OJ_TFLAGS は通常の =(再帰展開)で定義されています。make の変数展開の仕組み上、OJ_TLE が後から環境変数で上書きされた場合でも OJ_TFLAGS は正しく展開されるため実害はありませんが、OJ_TFLAGS:=(即時展開)ではなく = のままにしておく理由を明示するコメントがあると、将来の変更者が誤って := に変えてしまうリスクを防げます。

また、OJ_TLE に非整数値(例: OJ_TLE=abc)が渡された場合、oj がエラーを返しますが make 側では検知できません。利用者向けドキュメント(CLAUDE.md)に「秒数は正の整数で指定」と一言添えると親切です。

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ご指摘ありがとうございます。2点とも対応しました(ce00cf3)。

  • OJ_TFLAGS=(再帰展開)のままにする理由を makefile にコメントで明示し、将来 := に変えられて環境変数の上書きが効かなくなるのを防ぐようにしました。
  • OJ_TLE は正の整数の秒数である旨を makefile のコメントと CLAUDE.md の両方に追記しました。

…pansion

- Note OJ_TLE must be a positive integer (seconds) in makefile and CLAUDE.md
- Document why OJ_TFLAGS stays recursively expanded so env override works

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

全体的に設計は明確で、変更内容も適切です。1点、入力バリデーションに関する懸念を指摘します。

Comment thread lib/.support/makefile
# standard limit is 2s, but the dev container can be slower than the judge,
# so allow extra headroom to avoid false TLE. Override per task with e.g.
# `OJ_TLE=2 am t .java`.
OJ_TLE ?= 4

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ [MEDIUM] OJ_TLE は環境変数で上書き可能ですが、ユーザーが OJ_TLE=0OJ_TLE=-1、あるいは OJ_TLE=abc のような不正な値を渡した場合、oj がエラーを出すか、あるいは意図しない動作をする可能性があります。

Makefile 側で簡単なガードを入れることを検討してください。例えば:

ifneq ($(filter-out $(OJ_TLE),$(shell echo '$(OJ_TLE)' | grep -E '^[1-9][0-9]*$')),)
$(error OJ_TLE must be a positive integer, got: $(OJ_TLE))
endif

あるいは、CLAUDE.md のコメントに「正の整数で指定」と記載されているので、少なくともドキュメントに「不正な値を渡した場合の動作は未定義」と明記するだけでも親切です。現状では OJ_TLE=0 を渡すと oj--tle 0 を受け取り、全ケースが即座に TLE になる可能性があります。

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ご指摘ありがとうございます。makefile にガードを追加しました(4880a23)。

OJ_TLE が正の整数でない場合(0・負数・非数値)は $(error) で明確に停止するようにしました:

ifeq ($(shell echo '$(OJ_TLE)' | grep -Eq '^[1-9][0-9]*$$' && echo ok),)
$(error OJ_TLE must be a positive integer number of seconds, got: '$(OJ_TLE)')
endif

コンテナで動作確認済みです:

  • OJ_TLE=2(正常)→ --tle 2 で実行
  • OJ_TLE=0 / OJ_TLE=abc / OJ_TLE=-1(不正)→ makefile:143: *** OJ_TLE must be a positive integer number of seconds, got: '...'. Stop. で停止

これにより OJ_TLE=0 で全ケース即 TLE になる挙動や、oj 内部での分かりにくいエラーを未然に防げます。

Guard against invalid OJ_TLE values (0, negatives, non-numeric) with a
clear make $(error) instead of passing a bad value to oj. OJ_TLE=0 would
otherwise make every case TLE, and OJ_TLE=abc would fail deep inside oj.
Copilot AI review requested due to automatic review settings June 23, 2026 07:16

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

全体的に実装は明確で、バリデーションやドキュメントも丁寧に書かれています。1点、技術的に注意すべき点があります。

Comment thread lib/.support/makefile
endif
# Keep OJ_TFLAGS recursively expanded (`=`, not `:=`) so an env override of
# OJ_TLE is honoured at use time.
OJ_TFLAGS = -c '$(RUN_TEST)' --ignore-spaces-and-newline --tle $(OJ_TLE)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ [MEDIUM] コメントに「Keep OJ_TFLAGS recursively expanded (=, not :=) so an env override of OJ_TLE is honoured at use time.」とありますが、この説明は実際には不要(あるいは誤解を招く)です。

OJ_TLE ?= 4即時展開(simply expanded)ではなく遅延展開ですが、?= は最初の評価時に値が確定します。また OJ_TFLAGS=(再帰展開)にしても、$(OJ_TLE)OJ_TFLAGS展開される時点の値を使うため、OJ_TLE が環境変数で上書きされていれば ?= の時点でそちらが優先されます(?= は未定義の場合のみ代入するため)。

つまり = vs := の違いはこのケースでは実質的に影響しません。コメントが誤った理由付けをしているため、将来のメンテナンス時に混乱を招く可能性があります。コメントを削除するか、「OJ_TLE ?= により環境変数が優先されるため、OJ_TFLAGS の展開方式は影響しない」と正確に書き直すことを推奨します。

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ご指摘の通りです。OJ_TLE ?= 4 はパース時に評価され、環境変数があればその時点で優先されるため、OJ_TFLAGS=(再帰展開)でも :=(即時展開)でも $(OJ_TLE) は同じ値に解決されます。= vs := の違いはこのケースでは影響せず、コメントの理由付けが誤解を招くものでした。

誤った理由のコメントを残すより削除する方が良いと判断し、当該コメントを削除しました(b54e775)。OJ_TFLAGS = は元々この makefile のスタイルであり、特に正当化のコメントは不要です。

※ 先の LOW スレッドで「= のままにする理由を明示した」と返信していましたが、その理由付け自体が不正確でした。本コミットで訂正しています。

The comment claimed the recursive (=) expansion was needed for env
overrides, but OJ_TLE ?= resolves the env value at parse time, so = vs :=
makes no difference here. Remove the misleading rationale.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

全体的に設計は明確で、バリデーションの追加やドキュメントの更新も適切です。1点、シェルコマンドの実行方法に関して重要な問題があります。

Comment thread lib/.support/makefile Outdated
OJ_TLE ?= 4
# Fail fast on a non-positive-integer OJ_TLE instead of handing a bad value to
# oj (e.g. OJ_TLE=0 would make every case TLE, OJ_TLE=abc errors deep in oj).
ifeq ($(shell echo '$(OJ_TLE)' | grep -Eq '^[1-9][0-9]*$$' && echo ok),)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 [HIGH] [HIGH] $(shell ...) 内でのシングルクォートのエスケープ問題

ifeq ($(shell echo '$(OJ_TLE)' | grep -Eq '^[1-9][0-9]*$$' && echo ok),)

$(OJ_TLE) にシングルクォートが含まれる場合(例: OJ_TLE=1')、シェルコマンドが構文エラーになります。また、$(OJ_TLE) の展開がシェルに渡る前に Make によって処理されるため、意図しない展開が起きる可能性があります。

より堅牢な書き方として、printf '%s' を使うか、あるいは Make の filter 関数だけで完結させる方法が安全です:

# Make の組み込み関数のみで検証(シェル不要)
_OJ_TLE_CHECK := $(filter-out $(OJ_TLE),$(filter [1-9] [1-9][0-9] [1-9][0-9][0-9] [1-9][0-9][0-9][0-9],$(OJ_TLE)))
ifneq ($(_OJ_TLE_CHECK),)
$(error OJ_TLE must be a positive integer number of seconds, got: '$(OJ_TLE)')
endif

ただし filter はパターンマッチが限定的なため、実用上は grep を使う場合でもシングルクォートを避けて printf 経由にするのが安全です:

ifeq ($(shell printf '%s' '$(OJ_TLE)' | grep -Eq '^[1-9][0-9]*$$' && echo ok),)

さらに根本的には、OJ_TLE に任意の文字列が入ることを想定するなら、シェル展開を完全に避けるために Make の filter + findstring の組み合わせが最も安全です。

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ご指摘ありがとうございます。シェルを完全に排し、Make 組み込み関数のみで検証するように書き換えました(dc4a013)。

$(shell) は文字列をシェルに渡す仕様上、$(OJ_TLE) を埋め込む限りクォート破壊やインジェクションの余地が残ります。また export OJ_TLE + $$OJ_TLE(環境変数経由)も検討しましたが、?= のデフォルト値(環境変数なしの場合の 4)がパース時の $(shell) に伝わらず make がエラーで停止してしまい、通常の am t が壊れるため不採用としました。

最終的にシェル不要の純 Make 版にしています:

# Strip every digit: a non-empty remainder means a non-digit char is present.
OJ_TLE_NONDIGIT := $(strip $(subst 0,,$(subst 1,,...$(OJ_TLE)...)))
# Strip zeros: an empty remainder means the value is empty or all-zero.
OJ_TLE_NONZERO := $(strip $(subst 0,,$(OJ_TLE)))
ifneq ($(OJ_TLE_NONDIGIT),)
$(error OJ_TLE must be a positive integer number of seconds, got: '$(OJ_TLE)')
endif
ifeq ($(OJ_TLE_NONZERO),)
$(error OJ_TLE must be a positive integer number of seconds, got: '$(OJ_TLE)')
endif

コンテナ実機で確認済みです: default/4/2 は通過、0/00/-1/abc/1.5/空文字 は $(error) で停止。クォートやメタ文字を含む値でもシェルに渡らないため、インジェクションは原理的に発生しません。

Comment thread lib/.support/makefile Outdated
OJ_TLE ?= 4
# Fail fast on a non-positive-integer OJ_TLE instead of handing a bad value to
# oj (e.g. OJ_TLE=0 would make every case TLE, OJ_TLE=abc errors deep in oj).
ifeq ($(shell echo '$(OJ_TLE)' | grep -Eq '^[1-9][0-9]*$$' && echo ok),)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ [LOW] [LOW] $$ のエスケープについて

grep -Eq '^[1-9][0-9]*$$'$$ は Make の変数展開を防ぐためのエスケープで、シェルには $ として渡されます。これは正しい記述ですが、$(shell ...) 内では $$$ に変換されてシェルに渡るため、正規表現の $(行末アンカー)として機能します。意図通りの動作ですが、コメントで明示しておくと後から読む人に親切です。

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ご指摘ありがとうございます。HIGH の指摘への対応でガードをシェル不要の純 Make 版に書き換えたため(dc4a013)、grep$$ 正規表現自体がなくなり、この $$ エスケープの論点は解消されました。

Replace the shell-based OJ_TLE guard with pure Make string functions so a
value containing quotes or shell metacharacters cannot break the command or
inject code. Verified in-container: default/4/2 pass, 0/abc/-1 are rejected.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

全体的に設計は明確で、バリデーションロジックも丁寧に実装されています。以下に気になる点を挙げます。

Comment thread lib/.support/makefile
# Validate with Make built-ins only (no shell), so a value containing quotes or
# shell metacharacters cannot break or inject into a command.
# Strip every digit: a non-empty remainder means a non-digit char is present.
OJ_TLE_NONDIGIT := $(strip $(subst 0,,$(subst 1,,$(subst 2,,$(subst 3,,$(subst 4,,$(subst 5,,$(subst 6,,$(subst 7,,$(subst 8,,$(subst 9,,$(OJ_TLE))))))))))))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ [MEDIUM] バリデーションロジックに潜在的な問題があります。

OJ_TLE_NONDIGIT の計算では 0 を除外しているため、OJ_TLE=0 の場合は OJ_TLE_NONDIGIT が空になり、最初の ifneq チェックをパスしてしまいます。その後 OJ_TLE_NONZERO のチェックで捕捉されるため最終的には正しく弾かれますが、コメントに「Strip every digit」と書いてあるにもかかわらず 0 が含まれていないのは混乱を招きます。

より明確にするには、OJ_TLE_NONDIGIT の計算に 0 も含めるべきです:

OJ_TLE_NONDIGIT := $(strip $(subst 0,,$(subst 1,,$(subst 2,,$(subst 3,,$(subst 4,,$(subst 5,,$(subst 6,,$(subst 7,,$(subst 8,,$(subst 9,,$(OJ_TLE))))))))))))

これにより「全桁を除去した残りが空でなければ非数字文字あり」というロジックが名前と一致し、OJ_TLE_NONZERO は純粋に「ゼロのみ・空」チェックとして機能します。

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

恐れ入りますが、こちらは事実誤認かと思います。現行の OJ_TLE_NONDIGIT(145行目)は先頭に $(subst 0,,...) を含んでおり、0 も除去しています:

OJ_TLE_NONDIGIT := $(strip $(subst 0,,$(subst 1,,$(subst 2,,$(subst 3,,$(subst 4,,$(subst 5,,$(subst 6,,$(subst 7,,$(subst 8,,$(subst 9,,$(OJ_TLE))))))))))))

ご提示いただいた「修正版」コードは現行コードと完全に同一です。したがって「Strip every digit」というコメントと実装は一致しており、変更は不要です。

役割分担としては、OJ_TLE_NONDIGIT が「非数字文字の有無」、OJ_TLE_NONZERO が「空・全ゼロ(=非正)」を担当しています。0 は前者を通過しますが後者で正しく弾かれます(コンテナ実機で 0/00$(error) 停止することを確認済み)。

Comment thread lib/.support/makefile
# Validate with Make built-ins only (no shell), so a value containing quotes or
# shell metacharacters cannot break or inject into a command.
# Strip every digit: a non-empty remainder means a non-digit char is present.
OJ_TLE_NONDIGIT := $(strip $(subst 0,,$(subst 1,,$(subst 2,,$(subst 3,,$(subst 4,,$(subst 5,,$(subst 6,,$(subst 7,,$(subst 8,,$(subst 9,,$(OJ_TLE))))))))))))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ [LOW] OJ_TLE ?= 4?= を使っているため環境変数による上書きが可能ですが、バリデーション変数 OJ_TLE_NONDIGITOJ_TLE_NONZERO:= で即時展開されています。

Make では := 変数はファイルの読み込み順に評価されるため、OJ_TLE ?= 4 の後に := で評価される現在の順序は正しく動作します。ただし、将来 OJ_TLE の定義位置が変わった場合に評価順の問題が生じる可能性があります。コメントでこの依存関係を明示しておくと保守性が上がります。

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ご指摘ありがとうございます。評価順への依存はご指摘の通りですが、OJ_TLE_NONDIGIT / OJ_TLE_NONZERO の2行は OJ_TLE ?= 4(139行目)の直後(145〜147行目)に配置しており、依存関係は配置上自明です。さらにコメントを足すとかえって冗長になると判断し、現状維持とさせてください。

Comment thread CLAUDE.md
- C++ は `am t .cpp` で常に `/judge` ディレクトリ方式(Judge 環境と同じ)でコンパイル・実行されます。問題ディレクトリに `a.out` が生成されないため、クリーンな開発環境を維持できます。
- Elixir は `am t .ex` で常に Mix release(judge 環境と同じ方式)でビルドされます。ビルド時間が短いため、開発効率とjudge環境との完全一致を両立できます。
- JavaScript は `am t .js` で常に64MBスタックサイズ(judge 環境と同じ)で実行されます。デフォルトのNode.jsスタック(約1MB)と異なり、再帰処理でのスタックオーバーフローを防ぎます。
- `am t` / `am tf` は `oj` の `--tle` により各テストケースを**デフォルト4秒**で打ち切ります(makefile の `OJ_TLE` 変数)。AtCoder の標準制限は2秒ですが、Dev Container は judge より遅い場合があるため誤TLEを避けて余裕を持たせています。無限ループや極端に遅いコードでもハングせず `TLE` と表示されます。問題ごとに変更する場合は `OJ_TLE=2 am t .java` のように環境変数で上書きできます(秒数は正の整数で指定)。

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ [LOW] ドキュメントの記述「秒数は正の整数で指定」は正確で良いですが、OJ_TLE に非常に大きな値(例: OJ_TLE=99999)を指定した場合の挙動についても一言触れると、ユーザーが誤って極端な値を設定した際の混乱を防げます。現状の実装では oj にそのまま渡されるため、oj 側の制限に依存することになります。

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ご提案ありがとうございます。巨大値(例: OJ_TLE=99999)は oj にそのまま渡され、単に各ケースの待ち時間が長くなるだけで挙動は自明です。ドキュメントが過度に冗長になるのを避けるため、ここは追記せず現状維持とさせてください。

@toshi0806 toshi0806 merged commit 9c59e2e into main Jun 23, 2026
1 check passed
@toshi0806 toshi0806 deleted the add-test-timeout branch June 23, 2026 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants