-
Notifications
You must be signed in to change notification settings - Fork 0
ScreenshotTest修正 #307
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ScreenshotTest修正 #307
Conversation
Walkthroughスクリーンショット テスト群が有効化され、16件のComposeプレビュー/スクリーンショットテストを追加。Gradle設定(ラッパー版変更、バージョンカタログにscreenshot-validation-api追記、依存追加)と生成された静的テスト報告書が含まれます。 Changes
Sequence Diagram(s)(該当条件を満たさないため、図は省略します) Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@AndroidApp/ui/build/reports/screenshotTest/preview/debug/css/base-style.css`:
- Around line 1-162: Remove the gitignore exception lines that re-include
reports under the ui module: delete the patterns '!/build/reports' and
'!/build/reports/screenshotTest' from AndroidApp/ui/.gitignore while keeping the
existing '/build/*' rule so all build outputs are ignored; after updating
.gitignore, remove any already-committed build artifacts from version control
(e.g., git rm --cached) and commit the change.
In `@AndroidApp/ui/build/reports/screenshotTest/preview/debug/index.html`:
- Around line 131-132: Replace the insecure Gradle link using HTTP with HTTPS in
the generated HTML: locate the anchor element that currently reads <a
href="http://www.gradle.org"> (the "Generated by ... <a
href="http://www.gradle.org">Gradle</a>" snippet) and change its href to use
"https://www.gradle.org" (and update any other identical occurrences) so the
static report avoids mixed content.
- Around line 116-119: HTML table cell uses a self-closing <td class="success"/>
which breaks layout; change it to a proper opening and closing td and ensure the
<a href="me.nya_n.notificationnotifier.ScreenshotTest.html">…</a> is placed
inside the same <td> (e.g., <td class="success"> <a
...>me.nya_n.notificationnotifier.ScreenshotTest</a> </td>), so the link is
contained within the table cell and the table markup is valid.
In `@AndroidApp/ui/build/reports/screenshotTest/preview/debug/js/report.js`:
- Around line 3-23: initTabs currently assumes the `#tabs` container and derived
structures exist and will throw if they don't; add a defensive guard at the
start of initTabs to check that document.getElementById('tabs') returned a
truthy element and that findTabs(container) produces a usable result (e.g.,
non-empty array) before assigning to tabs.tabs/titles/headers; if the container
or required derived values are missing, do an early return (false) to avoid
exceptions. Reference: initTabs, tabs, findTabs, findTitles, findHeaders,
tabs.select.
In
`@AndroidApp/ui/build/reports/screenshotTest/preview/debug/me.nya_n.notificationnotifier.html`:
- Around line 1-104: The HTML test report me.nya_n.notificationnotifier.html
under AndroidApp/ui/build/reports/screenshotTest/preview is a generated build
artifact and should be removed from version control; delete this committed file
(and any other files like base-style.css under build/) from the repo, add the
build/ directory to .gitignore (or ensure build/ is globally excluded) so
generated reports aren’t committed, and commit the removal and .gitignore
update; note the <thread> typo is generated by Gradle and should not be changed
in this repo.
🧹 Nitpick comments (1)
AndroidApp/README.md (1)
20-22: 入れ子リストのインデントを修正してください。
MD007の指摘どおり、ネストした箇条書きのインデントが不正です。Markdownのレンダリング崩れを防ぐため、2スペースのネストに揃えてください。♻️ 修正案
-- ⚠ Windows環境だと、`CreateProcess error=206, ファイル名または拡張子が長すぎます。`というエラーが出る可能性がある - ⚠ - - 諦めてwsl2等使おう +- ⚠ Windows環境だと、`CreateProcess error=206, ファイル名または拡張子が長すぎます。`というエラーが出る可能性がある + ⚠ + - 諦めてwsl2等使おう
|
|
||
| body { | ||
| margin: 0; | ||
| padding: 0; | ||
| font-family: sans-serif; | ||
| font-size: 12pt; | ||
| } | ||
|
|
||
| body, a, a:visited { | ||
| color: #303030; | ||
| } | ||
|
|
||
| #content { | ||
| padding-left: 50px; | ||
| padding-right: 50px; | ||
| padding-top: 30px; | ||
| padding-bottom: 30px; | ||
| } | ||
|
|
||
| #content h1 { | ||
| font-size: 160%; | ||
| margin-bottom: 10px; | ||
| } | ||
|
|
||
| #footer { | ||
| margin-top: 100px; | ||
| font-size: 80%; | ||
| white-space: nowrap; | ||
| } | ||
|
|
||
| #footer, #footer a { | ||
| color: #a0a0a0; | ||
| } | ||
|
|
||
| ul { | ||
| margin-left: 0; | ||
| } | ||
|
|
||
| h1, h2, h3 { | ||
| white-space: nowrap; | ||
| } | ||
|
|
||
| h2 { | ||
| font-size: 120%; | ||
| } | ||
|
|
||
| ul.tabLinks { | ||
| padding-left: 0; | ||
| padding-top: 10px; | ||
| padding-bottom: 10px; | ||
| overflow: auto; | ||
| min-width: 800px; | ||
| width: auto !important; | ||
| width: 800px; | ||
| } | ||
|
|
||
| ul.tabLinks li { | ||
| float: left; | ||
| height: 100%; | ||
| list-style: none; | ||
| padding-left: 10px; | ||
| padding-right: 10px; | ||
| padding-top: 5px; | ||
| padding-bottom: 5px; | ||
| margin-bottom: 0; | ||
| -moz-border-radius: 7px; | ||
| border-radius: 7px; | ||
| margin-right: 25px; | ||
| border: solid 1px #d4d4d4; | ||
| background-color: #f0f0f0; | ||
| behavior: url(css3-pie-1.0beta3.htc); | ||
| } | ||
|
|
||
| ul.tabLinks li:hover { | ||
| background-color: #fafafa; | ||
| } | ||
|
|
||
| ul.tabLinks li.selected { | ||
| background-color: #c5f0f5; | ||
| border-color: #c5f0f5; | ||
| } | ||
|
|
||
| ul.tabLinks a { | ||
| font-size: 120%; | ||
| display: block; | ||
| outline: none; | ||
| text-decoration: none; | ||
| margin: 0; | ||
| padding: 0; | ||
| } | ||
|
|
||
| ul.tabLinks li h2 { | ||
| margin: 0; | ||
| padding: 0; | ||
| } | ||
|
|
||
| div.tab { | ||
| } | ||
|
|
||
| div.selected { | ||
| display: block; | ||
| } | ||
|
|
||
| div.deselected { | ||
| display: none; | ||
| } | ||
|
|
||
| div.tab table { | ||
| min-width: 350px; | ||
| width: auto !important; | ||
| width: 350px; | ||
| border-collapse: collapse; | ||
| } | ||
|
|
||
| div.tab th, div.tab table { | ||
| border-bottom: solid #d0d0d0 1px; | ||
| } | ||
|
|
||
| div.tab th { | ||
| text-align: left; | ||
| white-space: nowrap; | ||
| padding-left: 6em; | ||
| } | ||
|
|
||
| div.tab th:first-child { | ||
| padding-left: 0; | ||
| } | ||
|
|
||
| div.tab td { | ||
| white-space: nowrap; | ||
| padding-left: 6em; | ||
| padding-top: 5px; | ||
| padding-bottom: 5px; | ||
| } | ||
|
|
||
| div.tab td:first-child { | ||
| padding-left: 0; | ||
| } | ||
|
|
||
| div.tab td.numeric, div.tab th.numeric { | ||
| text-align: right; | ||
| } | ||
|
|
||
| span.code { | ||
| display: inline-block; | ||
| margin-top: 0em; | ||
| margin-bottom: 1em; | ||
| } | ||
|
|
||
| span.code pre { | ||
| font-size: 11pt; | ||
| padding-top: 10px; | ||
| padding-bottom: 10px; | ||
| padding-left: 10px; | ||
| padding-right: 10px; | ||
| margin: 0; | ||
| background-color: #f7f7f7; | ||
| border: solid 1px #d0d0d0; | ||
| min-width: 700px; | ||
| width: auto !important; | ||
| width: 700px; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check .gitignore for build directory exclusions
echo "=== .gitignore content ==="
if [ -f .gitignore ]; then
cat .gitignore
else
echo ".gitignore not found in root"
fi
echo ""
echo "=== Check for .gitignore in AndroidApp directory ==="
if [ -f AndroidApp/.gitignore ]; then
cat AndroidApp/.gitignore
else
echo "AndroidApp/.gitignore not found"
fi
echo ""
echo "=== Check for .gitignore in AndroidApp/ui directory ==="
if [ -f AndroidApp/ui/.gitignore ]; then
cat AndroidApp/ui/.gitignore
else
echo "AndroidApp/ui/.gitignore not found"
fi
echo ""
echo "=== Git-tracked files in build directories ==="
git ls-files 'AndroidApp/**/build/**' 2>/dev/null | head -20 || echo "No files found or git command failed"
echo ""
echo "=== Check if the specific CSS file is tracked ==="
git ls-files 'AndroidApp/ui/build/reports/screenshotTest/preview/debug/css/base-style.css' 2>/dev/nullRepository: ptkNktq/AndroidNotificationNotifier
Length of output: 4587
AndroidApp/ui/.gitignore のビルド成果物除外ルールを修正してください
AndroidApp/ui/build/reports/screenshotTest/ ディレクトリ内のファイルがバージョン管理にコミットされています。このディレクトリは Gradle によって自動生成されるスクリーンショットテストレポートで、ビルド成果物として扱うべきです。
AndroidApp/ui/.gitignore で以下のルールが存在します:
/build/*- build ディレクトリを除外!/build/reports- reports を例外として含める!/build/reports/screenshotTest- screenshotTest を例外として含める
これらの例外パターンを削除し、すべての build 成果物をバージョン管理から除外してください。
🧰 Tools
🪛 Biome (2.1.2)
[error] 54-54: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
width is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 111-111: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
width is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 161-161: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
width is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
🤖 Prompt for AI Agents
In `@AndroidApp/ui/build/reports/screenshotTest/preview/debug/css/base-style.css`
around lines 1 - 162, Remove the gitignore exception lines that re-include
reports under the ui module: delete the patterns '!/build/reports' and
'!/build/reports/screenshotTest' from AndroidApp/ui/.gitignore while keeping the
existing '/build/*' rule so all build outputs are ignored; after updating
.gitignore, remove any already-committed build artifacts from version control
(e.g., git rm --cached) and commit the change.
| <tr> | ||
| <td class="success"/> | ||
| <a href="me.nya_n.notificationnotifier.ScreenshotTest.html">me.nya_n.notificationnotifier.ScreenshotTest</a> | ||
| <td>16</td> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
td の自己終了タグでレイアウトが崩れます。
<td> を正しく開閉してリンクをセル内に収めてください。
🛠 修正案
-<td class="success"/>
-<a href="me.nya_n.notificationnotifier.ScreenshotTest.html">me.nya_n.notificationnotifier.ScreenshotTest</a>
+<td class="success">
+ <a href="me.nya_n.notificationnotifier.ScreenshotTest.html">me.nya_n.notificationnotifier.ScreenshotTest</a>
+</td>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <tr> | |
| <td class="success"/> | |
| <a href="me.nya_n.notificationnotifier.ScreenshotTest.html">me.nya_n.notificationnotifier.ScreenshotTest</a> | |
| <td>16</td> | |
| <tr> | |
| <td class="success"> | |
| <a href="me.nya_n.notificationnotifier.ScreenshotTest.html">me.nya_n.notificationnotifier.ScreenshotTest</a> | |
| </td> | |
| <td>16</td> |
🤖 Prompt for AI Agents
In `@AndroidApp/ui/build/reports/screenshotTest/preview/debug/index.html` around
lines 116 - 119, HTML table cell uses a self-closing <td class="success"/> which
breaks layout; change it to a proper opening and closing td and ensure the <a
href="me.nya_n.notificationnotifier.ScreenshotTest.html">…</a> is placed inside
the same <td> (e.g., <td class="success"> <a
...>me.nya_n.notificationnotifier.ScreenshotTest</a> </td>), so the link is
contained within the table cell and the table markup is valid.
| <p>Generated by | ||
| <a href="http://www.gradle.org">Gradle 8.14.3</a> at Jan 19, 2026, 6:41:25 AM</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gradleリンクは HTTPS を使用してください。
静的レポートでも mixed content を避けたいです。
🔒 修正案
-<a href="http://www.gradle.org">Gradle 8.14.3</a>
+<a href="https://www.gradle.org">Gradle 8.14.3</a>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <p>Generated by | |
| <a href="http://www.gradle.org">Gradle 8.14.3</a> at Jan 19, 2026, 6:41:25 AM</p> | |
| <p>Generated by | |
| <a href="https://www.gradle.org">Gradle 8.14.3</a> at Jan 19, 2026, 6:41:25 AM</p> |
🧰 Tools
🪛 ast-grep (0.40.5)
[warning] 131-131: "This link points to a plaintext HTTP URL. Prefer an encrypted HTTPS URL if possible."
Context: Gradle 8.14.3
Note: [CWE-319] Authentication Bypass by Primary Weakness [REFERENCES]
- https://cwe.mitre.org/data/definitions/319.html
(plaintext-http-link-html)
🤖 Prompt for AI Agents
In `@AndroidApp/ui/build/reports/screenshotTest/preview/debug/index.html` around
lines 131 - 132, Replace the insecure Gradle link using HTTP with HTTPS in the
generated HTML: locate the anchor element that currently reads <a
href="http://www.gradle.org"> (the "Generated by ... <a
href="http://www.gradle.org">Gradle</a>" snippet) and change its href to use
"https://www.gradle.org" (and update any other identical occurrences) so the
static report avoids mixed content.
| function initTabs() { | ||
| var container = document.getElementById('tabs'); | ||
| tabs.tabs = findTabs(container); | ||
| tabs.titles = findTitles(tabs.tabs); | ||
| tabs.headers = findHeaders(container); | ||
| tabs.select = select; | ||
| tabs.deselectAll = deselectAll; | ||
| // select tab if title param is provided in the URL | ||
| const urlParams = new URLSearchParams(window.location.search); | ||
| const titleParam = urlParams.get('title'); | ||
| if (titleParam) { | ||
| const index = tabs.titles.indexOf(titleParam); | ||
| if (index > -1) { | ||
| tabs.select(index); | ||
| return true; | ||
| } | ||
| } | ||
| // If no title or invalid title, select the first tab (default) | ||
| tabs.select(0); | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#tabs が存在しない場合のガードを追加してください。
レポートDOMの構成が変わると即時例外になるため、早期リターンで防御したいです。
🔧 変更案
function initTabs() {
- var container = document.getElementById('tabs');
+ var container = document.getElementById('tabs');
+ if (!container) {
+ return false;
+ }
tabs.tabs = findTabs(container);🤖 Prompt for AI Agents
In `@AndroidApp/ui/build/reports/screenshotTest/preview/debug/js/report.js` around
lines 3 - 23, initTabs currently assumes the `#tabs` container and derived
structures exist and will throw if they don't; add a defensive guard at the
start of initTabs to check that document.getElementById('tabs') returned a
truthy element and that findTabs(container) produces a usable result (e.g.,
non-empty array) before assigning to tabs.tabs/titles/headers; if the container
or required derived values are missing, do an early return (false) to avoid
exceptions. Reference: initTabs, tabs, findTabs, findTitles, findHeaders,
tabs.select.
| <!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www.w3.org/TR/html4/strict.dtd"> | ||
| <html> | ||
| <head> | ||
| <meta http-equiv="Content-Type" content="text/html; charset=utf-8"/> | ||
| <title>Test results - Package me.nya_n.notificationnotifier</title> | ||
| <link href="css/base-style.css" rel="stylesheet" type="text/css"/> | ||
| <link href="css/style.css" rel="stylesheet" type="text/css"/> | ||
| <script src="js/report.js" type="text/javascript"></script> | ||
| </head> | ||
| <body> | ||
| <div id="content"> | ||
| <h1>Package me.nya_n.notificationnotifier</h1> | ||
| <div class="breadcrumbs"> | ||
| <a href="index.html">all</a> > me.nya_n.notificationnotifier</div> | ||
| <div id="summary"> | ||
| <table> | ||
| <tr> | ||
| <td> | ||
| <div class="summaryGroup"> | ||
| <table> | ||
| <tr> | ||
| <td> | ||
| <div class="infoBox" id="tests"> | ||
| <div class="counter">16</div> | ||
| <p>tests</p> | ||
| </div> | ||
| </td> | ||
| <td> | ||
| <div class="infoBox" id="errors"> | ||
| <div class="counter">0</div> | ||
| <p>errors</p> | ||
| </div> | ||
| </td> | ||
| <td> | ||
| <div class="infoBox" id="failures"> | ||
| <div class="counter">0</div> | ||
| <p>failures</p> | ||
| </div> | ||
| </td> | ||
| <td> | ||
| <div class="infoBox" id="skipped"> | ||
| <div class="counter">0</div> | ||
| <p>skipped</p> | ||
| </div> | ||
| </td> | ||
| <td> | ||
| <div class="infoBox" id="duration"> | ||
| <div class="counter">3.573s</div> | ||
| <p>duration</p> | ||
| </div> | ||
| </td> | ||
| </tr> | ||
| </table> | ||
| </div> | ||
| </td> | ||
| <td> | ||
| <div class="infoBox success" id="successRate"> | ||
| <div class="percent">100%</div> | ||
| <p>successful</p> | ||
| </div> | ||
| </td> | ||
| </tr> | ||
| </table> | ||
| </div> | ||
| <div id="tabs"> | ||
| <ul class="tabLinks"> | ||
| <li> | ||
| <a href="#tab0">Classes</a> | ||
| </li> | ||
| </ul> | ||
| <div id="tab0" class="tab"> | ||
| <h2>Classes</h2> | ||
| <table> | ||
| <thread> | ||
| <tr> | ||
| <th>Class</th> | ||
| <th>Tests</th> | ||
| <th>Errors</th> | ||
| <th>Failures</th> | ||
| <th>Skipped</th> | ||
| <th>Duration</th> | ||
| <th>Success rate</th> | ||
| </tr> | ||
| </thread> | ||
| <tr> | ||
| <td class="success"> | ||
| <a href="me.nya_n.notificationnotifier.ScreenshotTest.html">ScreenshotTest</a> | ||
| </td> | ||
| <td>16</td> | ||
| <td>0</td> | ||
| <td>0</td> | ||
| <td>0</td> | ||
| <td>3.573s</td> | ||
| <td class="success">100%</td> | ||
| </tr> | ||
| </table> | ||
| </div> | ||
| </div> | ||
| <div id="footer"> | ||
| <p>Generated by | ||
| <a href="http://www.gradle.org">Gradle 8.14.3</a> at Jan 19, 2026, 6:41:25 AM</p> | ||
| </div> | ||
| </div> | ||
| </body> No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ビルド成果物がバージョン管理にコミットされています
このHTMLファイルもGradleによって自動生成されたテストレポートです。base-style.css と同様に、build/ ディレクトリ全体をバージョン管理から除外してください。
なお、74行目に <thread> というタイポがありますが(正しくは <thead>)、これはGradle自体が生成したコードであり、このリポジトリでは修正できません。
🧰 Tools
🪛 ast-grep (0.40.5)
[warning] 100-100: "This link points to a plaintext HTTP URL. Prefer an encrypted HTTPS URL if possible."
Context: Gradle 8.14.3
Note: [CWE-319] Authentication Bypass by Primary Weakness [REFERENCES]
- https://cwe.mitre.org/data/definitions/319.html
(plaintext-http-link-html)
🤖 Prompt for AI Agents
In
`@AndroidApp/ui/build/reports/screenshotTest/preview/debug/me.nya_n.notificationnotifier.html`
around lines 1 - 104, The HTML test report me.nya_n.notificationnotifier.html
under AndroidApp/ui/build/reports/screenshotTest/preview is a generated build
artifact and should be removed from version control; delete this committed file
(and any other files like base-style.css under build/) from the repo, add the
build/ directory to .gitignore (or ensure build/ is globally excluded) so
generated reports aren’t committed, and commit the removal and .gitignore
update; note the <thread> typo is generated by Gradle and should not be changed
in this repo.
Summary by CodeRabbit
新機能
ドキュメント
Chores
✏️ Tip: You can customize this high-level summary in your review settings.