Fix/diagnostics test env assertion by machida · Pull Request #113 · machida/mb

machida · 2025-12-03T01:41:06Z

Summary by CodeRabbit

リリースノート

テスト
- テスト選択子をより堅牢な仕様ベースのセレクタに更新し、テストの安定性を向上させました。
バグ修正
- 診断機能の環境判定をより正確に改善しました。
スタイル
- Tailwind設定のメディアクエリを最適化しました。

_{✏️ Tip: You can customize this high-level summary in your review settings.}

@source

Problem: - Tailwind utility classes (text-white, flex, etc.) were not being generated - CSS output only included custom component classes, missing core utilities Root causes: 1. @source glob pattern **/*.{html,erb} did not match .html.erb files 2. @theme directive without explicit utilities import suppressed defaults 3. @Custom-Media (CSS draft feature) caused build errors in Tailwind v4 Changes: - Fix @source patterns to properly scan template files: - **/*.{html,erb} → **/*.erb (matches .html.erb files) - Remove ./ prefix for clarity (paths are from project root) - Scope to essential directories only - Remove @theme import to restore default Tailwind utilities - Replace @Custom-Media with standard media queries: - @media (--breakpoint-lg) → @media (min-width: 1024px) - @media (--breakpoint-md) → @media (min-width: 768px) Result: - Utility classes now generated correctly (56KB → 147KB CSS) - All Tailwind build tests passing - text-white, flex, and other utilities available in views

Problem: - Diagnostics controller returned "development" for all non-production environments - Test assertions expected "development" even when running in test environment - This created confusion about which environment was actually being checked Changes: - Update gcs_check method to return actual environment name (Rails.env.to_s) - Update message to interpolate actual environment: "GCS not required in #{Rails.env}" - Update test assertions to expect "test" environment values instead of "development" Result: - GCS diagnostics now accurately reflect the current environment - Test environment returns "test" status and message - Development environment returns "development" status and message - All diagnostics controller tests passing

Updated all Playwright system tests to use semantic .spec-- class selectors instead of HTML tag and attribute selectors for improved test stability and maintainability. Changes: - admin_management_playwright_test.rb: password form fields and title - admin_site_settings_playwright_test.rb: site settings form fields - author_display_toggle_playwright_test.rb: site settings title - copyright_edge_cases_playwright_test.rb: site settings title, copyright input, save button - copyright_rewrite_bug_playwright_test.rb: site settings title, copyright input, save button - copyright_update_playwright_test.rb: site settings title, copyright input, save button Replaced selectors: - h1:has-text('サイト設定') → .spec--site-settings-title - h1:has-text('パスワード変更') → .spec--password-edit-title - input[name='site_settings[site_title]'] → .spec--site-title-input - input[name='site_settings[copyright]'] → .spec--copyright-input - textarea[name='site_settings[top_page_description]'] → .spec--top-page-description-input - input[name='admin[password]'] → .spec--password-input - input[name='admin[password_confirmation]'] → .spec--password-confirmation-input - input[type='submit'][value='設定を保存'] → .spec--save-button - input[type='submit'][value='パスワードを変更'] → .spec--password-change-button All affected tests verified to pass with new selectors. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>

coderabbitai · 2025-12-03T01:41:16Z

Walkthrough

CSSのカスタムメディアクエリ変数を明示的なピクセル値に置換し、Tailwindの@sourceディレクティブを幅広いグロブから明示的なパスに絞り込み、_theme.cssのインポートを削除。診断コントローラーのgcs_checkを静的なステータスから動的なRails.envベースに更新。複数のシステムテストをBEM形式の.spec--クラスセレクタに移行。

Changes

Cohort / File(s)	Summary
CSSアーキテクチャの最適化 `app/assets/tailwind/application.css`, `app/assets/tailwind/components/_action-banner.css`, `app/assets/tailwind/components/_admin-nav.css`	Tailwindの`@source`ディレクティブを、幅広いグロブ（`app/*/.{erb,rb,ts,js}`）から明示的なパス（`app/views/*/.erb`、`app/components/*/.rb`、`app/helpers/*/.rb`、`app/javascript/*/.js`）に変更。`_theme.css`インポートを削除。カスタムメディアクエリ変数（`@media (--breakpoint-md)`、`@media (--breakpoint-lg)`）を明示的なピクセル値（`min-width: 768px`、`min-width: 1024px`）に置換。
バックエンド診断ロジックの動的化 `app/controllers/admin/diagnostics_controller.rb`, `test/controllers/admin/diagnostics_controller_test.rb`	`gcs_check`メソッドの非本番環境ブランチで、ハードコードされた`"development"`ステータスと静的メッセージを、実際の`Rails.env`を反映した動的ステータスとメッセージに更新。テスト期待値を`"test"`と対応するメッセージに調整。
Playwrightテストセレクタ統一化 `test/system/admin_management_playwright_test.rb`, `test/system/admin_site_settings_playwright_test.rb`, `test/system/author_display_toggle_playwright_test.rb`, `test/system/copyright_edge_cases_playwright_test.rb`, `test/system/copyright_rewrite_bug_playwright_test.rb`, `test/system/copyright_update_playwright_test.rb`	属性ベースのセレクタ（`input[name='...']`、`h1:has-text('...')`、`input[type='submit'][value='...']`）をBEM形式の`.spec--`クラスセレクタ（`.spec--site-settings-title`、`.spec--copyright-input`、`.spec--save-button`、`.spec--password-input`など）に統一。パスワード変更フローのテストではDOM/テキストベースセレクタを仕様ベースのセレクタに置換。

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

レビューの重点項目：

CSSブレークポイント置換の一貫性確認: 複数ファイルにおけるメディアクエリ値の置換が、すべてのレスポンシブレイアウトで意図した動作を保つか確認（768px、1024pxの選択根拠が既存のカスタム変数定義と正確に対応しているか）
Tailwind @sourceパスの網羅性: 新しい明示的なパスが、除外されたファイル（例：ビューのパーシャル、カスタムコンポーネント）を誤りなくカバーしているか確認
_theme.css削除の影響範囲: 削除されたテーマインポートに依存していたスタイル定義が他のCSSファイルで代替実装されているか確認
gcs_checkのRails.env動的化: 本番環境と非本番環境での分岐ロジックが想定通りに動作するか、ステータス値の型と使用個所を確認
テストセレクタの安定性: .spec--クラスセレクタがビューテンプレートで正確に定義されており、HTMLリファクタリングに対して堅牢か確認

Possibly related PRs

PR #66: CSSモジュール化の一部を取り消し、同じコンポーネントファイルの_admin-nav.cssと_action-banner.cssでカスタムメディアクエリ変数を明示的なmin-width値に置換する直接の関連性
PR #52: app/controllers/admin/diagnostics_controller.rbを初期導入したPRであり、本PRがgcs_checkメソッドの動的化で拡張する直接の前提PR
PR #6: システムテストの.spec--プレフィックス付きセレクタ規約とPlaywright移行を実装したPRであり、本PRが複数のシステムテストでそのセレクタ規約を採用する直接の関連先

Poem

ブレークポイント明示して 🎯
セレクタ統一に 🎨
テスト堅牢、カスタム変数は離別
CSSクリーンに Rails.envは動的に
保守性高し、ベストプラクティス✨

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	PRのタイトルは「Fix/diagnostics test env assertion」であり、実際の変更内容（CSS設定の明示的化、メディアクエリの置き換え、テストセレクタの統一化など多岐にわたる）を部分的にしか反映していません。タイトルは診断コントローラーのテスト環境アサーションの修正に言及していますが、これはPR全体の変更の一部に過ぎません。	タイトルを「Refactor CSS sources, media queries, and test selectors for better maintainability」など、より包括的な変更内容を反映したものに修正してください。
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch fix/diagnostics-test-env-assertion

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 and usage tips.}

coveralls · 2025-12-03T01:43:56Z

Pull Request Test Coverage Report for Build 19879444109

Details

1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 76.523%

Totals
Change from base Build 19879024902:	0.0%
Covered Lines:	644
Relevant Lines:	818

💛 - Coveralls

coderabbitai

Actionable comments posted: 0

Caution

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

⚠️ Outside diff range comments (1)

test/system/copyright_edge_cases_playwright_test.rb (1)
168-209: copyright_selector_after が未定義で NameError になります

テスト "copyright change with page navigation in between" の終盤で、
copyright_field = @page.locator(copyright_selector_after)
としていますが、copyright_selector_after というローカル変数はこのテスト内・クラス内どこにも定義されておらず、実行時に NameError でテストが必ず落ちる状態です。

このケースでは直前まで .spec--copyright-input を前提に扱っているので、単純に同じセレクタを使う形に修正するのが安全だと思います。
-  copyright_field = @page.locator(copyright_selector_after)
+  copyright_field = @page.locator(".spec--copyright-input")
または、他テスト同様に copyright_selector 変数を導入して使い回す場合は、その変数を事前に適切にセットする処理を追加してください。

🧹 Nitpick comments (3)

test/controllers/admin/diagnostics_controller_test.rb (1)

39-47: テスト名と期待値の環境表現を揃えると読みやすくなります

json["gcs_configured"]["status"] / message の期待値を "test" / "GCS not required in test" に更新したのは、Rails.env ベースの実装と整合していて良いと思います。一方でテスト名が "in development" のままなので、"in test environment" などにリネームしておくと、後から読んだときに意図がより明確になります。

Based on learnings, test/**/*test.rb の命名から対象環境が分かるとメンテナンス性が上がります。

test/system/copyright_update_playwright_test.rb (1)

19-75: .spec-- セレクタへの移行は妥当で、フォールバックも安全です

submit_settings_and_wait の保存ボタンや、各テストでのタイトル・著作権入力フィールドを .spec--save-button / .spec--site-settings-title / .spec--copyright-input に揃えていて、テストの読みやすさと安定性が向上しています。COPYRIGHT_INPUT を優先しつつ、タイムアウト時に .spec--copyright-input へフォールバックする構造も、マークアップ移行期間中の互換性という観点で合理的です。

将来的にビューが完全に .spec-- クラスに統一できた段階では、COPYRIGHT_INPUT まわりの begin/rescue を整理して .spec--copyright-input に一本化するとテストコードがさらに簡潔になりそうです。

test/system/copyright_edge_cases_playwright_test.rb (1)

18-26: エッジケース用テストの .spec-- セレクタ化は妥当です

複数パターン（シード値／年入り文字列／連続変更／ナビゲーション挟み／Unicode・絵文字／フッター反映）すべてで、サイト設定タイトル・著作権入力・保存ボタンを .spec--site-settings-title / .spec--copyright-input / .spec--save-button に揃えていて、他のシステムテストと同じセレクタ戦略になっています。COPYRIGHT_INPUT とのフォールバックも残してあり、テンプレート移行期間中の互換性確保という意味では良い折衷に見えます。

将来、ビューが完全に .spec-- クラスのみになるタイミングでは、COPYRIGHT_INPUT との二重管理をやめて .spec--copyright-input 前提に整理すると、このファイルも含めてテストコードがかなり読みやすくなりそうです。

Also applies to: 37-47, 50-65, 75-87, 93-109, 119-129, 131-166, 168-209, 219-263, 273-283, 287-302

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f4076d9 and 0df171f.

📒 Files selected for processing (11)

app/assets/tailwind/application.css (1 hunks)
app/assets/tailwind/components/_action-banner.css (3 hunks)
app/assets/tailwind/components/_admin-nav.css (5 hunks)
app/controllers/admin/diagnostics_controller.rb (1 hunks)
test/controllers/admin/diagnostics_controller_test.rb (1 hunks)
test/system/admin_management_playwright_test.rb (2 hunks)
test/system/admin_site_settings_playwright_test.rb (1 hunks)
test/system/author_display_toggle_playwright_test.rb (2 hunks)
test/system/copyright_edge_cases_playwright_test.rb (13 hunks)
test/system/copyright_rewrite_bug_playwright_test.rb (5 hunks)
test/system/copyright_update_playwright_test.rb (6 hunks)

🧰 Additional context used

📓 Path-based instructions (6)

**/*.css

📄 CodeRabbit inference engine (CLAUDE.md)

Use rem units instead of pixels in CSS, remove blank lines beyond 2 consecutive lines, and trim trailing whitespace.

Files:

app/assets/tailwind/components/_action-banner.css
app/assets/tailwind/components/_admin-nav.css
app/assets/tailwind/application.css

**/*.{rb,js,ts,jsx,tsx,erb,html,css}

📄 CodeRabbit inference engine (CLAUDE.md)

Ensure files end with a newline character and contain no trailing whitespace.

Files:

app/assets/tailwind/components/_action-banner.css
test/system/copyright_update_playwright_test.rb
test/system/admin_management_playwright_test.rb
app/assets/tailwind/components/_admin-nav.css
app/controllers/admin/diagnostics_controller.rb
test/system/author_display_toggle_playwright_test.rb
test/system/copyright_rewrite_bug_playwright_test.rb
test/system/copyright_edge_cases_playwright_test.rb
test/system/admin_site_settings_playwright_test.rb
app/assets/tailwind/application.css
test/controllers/admin/diagnostics_controller_test.rb

**/*.rb

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.rb: Follow Rails Omakase RuboCop standards with 2-space indentation, snake_case naming, and Admin:: namespace convention. Run bundle exec rubocop regularly and use -a/-A flags only when necessary.
Limit class files to 200 lines or less. Split larger classes into smaller, single-responsibility modules and extract business logic to app/services or lib/.

Files:

test/system/copyright_update_playwright_test.rb
test/system/admin_management_playwright_test.rb
app/controllers/admin/diagnostics_controller.rb
test/system/author_display_toggle_playwright_test.rb
test/system/copyright_rewrite_bug_playwright_test.rb
test/system/copyright_edge_cases_playwright_test.rb
test/system/admin_site_settings_playwright_test.rb
test/controllers/admin/diagnostics_controller_test.rb

test/**/*test.rb

📄 CodeRabbit inference engine (CLAUDE.md)

Name test files by feature (e.g., admin_creates_article_test.rb). Include at least one failure case per new feature test, using both unit tests and system/Playwright tests.

Files:

test/system/copyright_update_playwright_test.rb
test/system/admin_management_playwright_test.rb
test/system/author_display_toggle_playwright_test.rb
test/system/copyright_rewrite_bug_playwright_test.rb
test/system/copyright_edge_cases_playwright_test.rb
test/system/admin_site_settings_playwright_test.rb
test/controllers/admin/diagnostics_controller_test.rb

test/system/**/*test.rb

📄 CodeRabbit inference engine (CLAUDE.md)

Use .spec-- selectors in system/Playwright tests to query elements. When tests fail, attach traces using npx playwright show-report. Reference test config from test/test_helper.rb TestConfig class.

Files:

test/system/copyright_update_playwright_test.rb
test/system/admin_management_playwright_test.rb
test/system/author_display_toggle_playwright_test.rb
test/system/copyright_rewrite_bug_playwright_test.rb
test/system/copyright_edge_cases_playwright_test.rb
test/system/admin_site_settings_playwright_test.rb

app/assets/tailwind/application.css

📄 CodeRabbit inference engine (CLAUDE.md)

@source directive paths must be relative to Rails.root (project root), not relative to the CSS file. Use ./app/views/**/*.{html,erb} and app/components/**/*.{rb,erb} format.

Files:

app/assets/tailwind/application.css

🧠 Learnings (12)

📓 Common learnings

Learnt from: CR
Repo: machida/mb PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T01:19:04.764Z
Learning: Applies to **/*.{erb,html} : Use `.spec--` classes exclusively for testing selectors (Playwright/MiniTest), not for CSS styling. Update `test/support/selectors.rb` when adding or modifying `.spec--` classes.

Learnt from: CR
Repo: machida/mb PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T01:19:04.764Z
Learning: Applies to **/*.{erb,html} : Use Tailwind CSS utility classes or ViewComponents for styling. Extract repeated utility patterns into `app/components` to avoid duplication.

Learnt from: CR
Repo: machida/mb PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T01:19:04.764Z
Learning: Applies to app/assets/tailwind/application.css : `source` directive paths must be relative to Rails.root (project root), not relative to the CSS file. Use `./app/views/**/*.{html,erb}` and `app/components/**/*.{rb,erb}` format.

Learnt from: CR
Repo: machida/mb PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T01:19:04.764Z
Learning: Applies to test/system/**/*test.rb : Use `.spec--` selectors in system/Playwright tests to query elements. When tests fail, attach traces using `npx playwright show-report`. Reference test config from `test/test_helper.rb` `TestConfig` class.