Skip to content

test: 低価値な重複・自明テストを削減しメンテコストを下げる#464

Merged
yusuke0610 merged 2 commits into
mainfrom
test/remove-low-value-tests
Jul 5, 2026
Merged

test: 低価値な重複・自明テストを削減しメンテコストを下げる#464
yusuke0610 merged 2 commits into
mainfrom
test/remove-low-value-tests

Conversation

@yusuke0610

@yusuke0610 yusuke0610 commented Jul 5, 2026

Copy link
Copy Markdown
Owner

Summary

backend / frontend のテストコードを調査し、費用対効果の低いテストを削除・parametrize 統合した(テスト内容の質は高く、大規模な削除対象は無かった)。

  • web: web/tests/payloadBuilders.test.cjs(node:test)を削除。web/src/payloadBuilders.test.ts(vitest)の完全な部分集合で、Stryker(mutation テスト)にも寄与しないことを確認済み。test:unit の tsc 手動コンパイル → .test-dist 生成 → node:test の脆いビルド段取りも撤去し、npm testvitest run に単純化
  • backend: test_schemas.py の完全重複テスト1件を削除。Experience/Project の期間バリデーション10テストを共通契約として parametrize 統合(12ケース実行、契約は維持)
  • backend: test_messages.py の文言ベタ pin 3本を代表1本に集約(mutmut スコープ外・文言変更で壊れるだけで検出力ゼロ)
  • backend: auth/test_token_manager.py の JWTクレーム過分割4本を1本に統合(セキュリティ契約テストは維持)
  • backend: test_database_libsql_binary.py から SQLAlchemy 自体の挙動確認1件を削除

調査で「疑わしい」と当初想定した test_agent.py / test_llm_clients.py(境界モックのみで実ロジックを通す)、_extended 系ペア、useCareerExperienceMutators.test.ts 等は精査の結果高価値と判断し、変更していない。security/ ・auth/ ・test_error_code_sync.py 等の契約テストも維持。

Test plan

  • make ci が green(backend pytest 655 passed / web vitest 365 passed・53 files / build-web 成功)
  • 変更した backend テストファイル単体でも pass 確認済み
  • test:unit / .test-dist の参照が他ファイル(CI/Makefile/docs)に残っていないことを grep で確認
  • schemas/routers は変更していないため codegen-types は対象外

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved validation coverage for dates and token/message handling, helping prevent incorrect data from slipping through.
    • Refined database-related handling to better support binary values in supported environments.
  • Chores
    • Updated frontend test commands to run the main web test suite directly.
    • Cleaned up test and ignore settings so previously excluded files are now tracked and scanned as expected.
  • Documentation
    • Clarified frontend testing guidance and success criteria.

backend/frontend のテストコードを調査し、重複カバレッジ・逐語 pin・過分割の
低価値テストを削除・parametrize 統合した。web は node:test 経由の
payloadBuilders.test.cjs(vitest 版の完全な部分集合)を撤去し、
test:unit の脆いビルド段取りも合わせて廃止した。

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@github-actions github-actions Bot added test テスト追加・修正 documentation Improvements or additions to documentation backend バックエンド web フロントエンド (web) labels Jul 5, 2026
@coderabbitai

coderabbitai Bot commented Jul 5, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@yusuke0610, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 23 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 9c70938f-42dd-4e42-af88-aa75a7e13ba1

📥 Commits

Reviewing files that changed from the base of the PR and between dd3d75f and 1908548.

📒 Files selected for processing (1)
  • backend/tests/test_messages.py
📝 Walkthrough

Walkthrough

This PR consolidates test tooling and suites: web test scripts now run vitest exclusively (removing node:test-based dual invocation and a legacy CJS test file), test policy docs are updated accordingly, unused .test-dist ignore entries are dropped, and several backend pytest test files are refactored to combine/parametrize previously separate test cases.

Changes

Web Test Tooling Consolidation

Layer / File(s) Summary
Vitest-only test script and removal of legacy suite
web/package.json, web/tests/payloadBuilders.test.cjs
The test script now runs vitest run directly, removing test:unit/test:vitest composition and their script entries; the legacy payloadBuilders.test.cjs suite is deleted entirely.
Test policy docs and ignore-rule cleanup
.claude/rules/web/test.md, .gitignore, .jscpd.json
Test policy doc removes node:test references and updates wording to vitest-only guidance; .gitignore and .jscpd.json drop the now-unused web/.test-dist/ ignore entries.

Backend Test Suite Consolidation

Layer / File(s) Summary
Token manager claim test consolidation
backend/tests/auth/test_token_manager.py
Separate access/refresh token claim tests are merged into one test decoding both tokens (RS256) and asserting sub, exp, type, and jti claims.
libsql binary shim test cleanup
backend/tests/test_database_libsql_binary.py
Removes unused create_engine/NullPool imports and deletes the standard sqlite dialect binary test, keeping the libsql-specific dbapi.Binary shim test.
Message lookup test consolidation
backend/tests/test_messages.py
Combines separate notification/error key tests into test_key_lookup_returns_message; removes the test_validation_end_date_required test.
Schema period validation parametrization
backend/tests/test_schemas.py
Adds ProjectPeriod import and shared payload-builder helpers; replaces discrete Experience/Project end-date tests with parametrized tests covering ordering, in-progress normalization, and None rejection across both period types.

Estimated code review effort: 2 (Simple) | ~12 minutes

Possibly related PRs

  • yusuke0610/devforge#270: Both PRs touch backend centralized message handling and backend/tests/test_messages.py key-based lookups.
  • yusuke0610/devforge#277: Both PRs update backend/tests/test_schemas.py for ProjectPeriod/periods-based end-date validation.

Suggested labels: refactor

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main change: removing redundant, low-value tests to reduce maintenance cost.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch test/remove-low-value-tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai 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.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
backend/tests/test_messages.py (1)

34-34: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Add a direct assertion for validation.end_date_required The schema tests cover the missing-end-date branch, but they only match the general “終了年月” text. Keep one message-level assertion so get_error("validation.end_date_required") stays pinned to the expected string.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/tests/test_messages.py` at line 34, The message test is only covering
the generic end-date error text, so add a direct assertion in
test_master_data_placeholder for validation.end_date_required using
get_error("validation.end_date_required") to pin the exact expected string. Keep
the existing schema coverage, but make sure this test explicitly checks the
message-level key so the validation message contract stays stable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@backend/tests/test_messages.py`:
- Line 34: The message test is only covering the generic end-date error text, so
add a direct assertion in test_master_data_placeholder for
validation.end_date_required using get_error("validation.end_date_required") to
pin the exact expected string. Keep the existing schema coverage, but make sure
this test explicitly checks the message-level key so the validation message
contract stays stable.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 6665145a-aa12-42af-b3e9-a7dc84ffe8f8

📥 Commits

Reviewing files that changed from the base of the PR and between 6b868f4 and dd3d75f.

📒 Files selected for processing (9)
  • .claude/rules/web/test.md
  • .gitignore
  • .jscpd.json
  • backend/tests/auth/test_token_manager.py
  • backend/tests/test_database_libsql_binary.py
  • backend/tests/test_messages.py
  • backend/tests/test_schemas.py
  • web/package.json
  • web/tests/payloadBuilders.test.cjs
💤 Files with no reviewable changes (3)
  • .gitignore
  • web/tests/payloadBuilders.test.cjs
  • .jscpd.json

CodeRabbit 指摘: schemas/resume.py の model_validator から直接参照される
キーの完全文言 pin が統合で失われていた。文言ベタ pin の統合方針は維持しつつ、
実際に参照されているこのキーだけは代表テストに明示 assert を追加する。

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@yusuke0610
yusuke0610 merged commit ecef88f into main Jul 5, 2026
20 checks passed
@yusuke0610
yusuke0610 deleted the test/remove-low-value-tests branch July 20, 2026 12:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend バックエンド documentation Improvements or additions to documentation test テスト追加・修正 web フロントエンド (web)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant