ci: add test summary and upload failed unit/component test reports #1205
Conversation
I've been working on a PR to add "likes" as a new facet that can be visualized in what I'm calling "trends" charts (e.g. downloads over time, likes over time). That PR generalizes the existing "downloads" chart to support one or more given facets. I've had a half dozen painful merge conflicts across 37 locale files, so I've pulled out just this i18n key rename into its own PR to make this all easier.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
📝 WalkthroughWalkthroughThis PR updates the CI workflow and the i18n translations. The CI changes switch test runners to emit default, JUnit and HTML reporters, add test-summary rendering steps, and add conditional artifact uploads for unit, component and browser reports (naming: unit-test-report, component-test-report, playwright-report), while preserving existing coverage/upload steps. The UI changes move many download-related translation keys from Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
lunaria/files/zh-CN.json (1)
369-388:⚠️ Potential issue | 🟠 MajorAdd missing
package.trendskeys to zh-CN.json to prevent runtime fallback.The zh-CN.json file is missing four keys that exist in en-US.json under
package.trends:items.downloads,legend_estimation,no_data, andy_axis_label. These missing keys will cause the application to fall back to untranslated raw key strings at runtime. Add Chinese translations for these four keys to achieve parity with the base locale.No stale references to old
package.downloads.*keys were found in the codebase.app/pages/compare.vue (1)
241-246:⚠️ Potential issue | 🟡 MinorAdd
compare.facets.trends.titleto all locales.The new heading now relies on
compare.facets.trends.title; several provided locale files (zh-TW, ru-RU, ja-JP, it-IT and Lunaria it-IT) don’t show this key yet. Please add translations there to avoid missing-key fallbacks at runtime/i18n checks.
🟡 Minor comments (8)
lunaria/files/hu-HU.json-305-316 (1)
305-316:⚠️ Potential issue | 🟡 MinorAdd missing
date_range_multilineunderpackage.trends.
date_range_multilineis absent here, which is likely to surface as a missing translation/fallback if the base locale defines it for the trends UI. Please add it to keep key parity.lunaria/files/te-IN.json-250-263 (1)
250-263:⚠️ Potential issue | 🟡 MinorAdd missing
package.trends.no_dataandpackage.trends.y_axis_labeltranslations.The new
package.trendsblock doesn’t include theno_dataandy_axis_labelstrings that were part of the downloads UI and are now expected under trends. This will cause missing‑key fallbacks for te‑IN if those keys are still referenced. Please add them here (or confirm their usage was removed everywhere).lunaria/files/fr-FR.json-349-367 (1)
349-367:⚠️ Potential issue | 🟡 MinorAdd labels for any additional trend facets.
Line 365–367 only definesdownloads. If the trends chart now supports other facets (e.g., likes), those labels will otherwise fall back to raw keys in fr-FR.Suggested addition
"items": { - "downloads": "Téléchargements" + "downloads": "Téléchargements", + "likes": "Likes" }i18n/locales/zh-CN.json-375-387 (1)
375-387:⚠️ Potential issue | 🟡 MinorAdd missing
package.trendsstrings to prevent fallback.
Please add translations forno_data,y_axis_label,legend_estimation, anditems.downloadsso the trends UI doesn’t fall back to English.lunaria/files/es-419.json-379-392 (1)
379-392:⚠️ Potential issue | 🟡 MinorAdd missing
package.trendsstrings to avoid English fallback.
The new trends UI introducesno_data,y_axis_label,legend_estimation, anditems.downloadsin other locales; add equivalents here so this locale doesn’t fall back to English or raw keys.i18n/locales/es.json-379-392 (1)
379-392:⚠️ Potential issue | 🟡 MinorAdd missing
package.trendsstrings to avoid fallback text.
no_data,y_axis_label,legend_estimation, anditems.downloadsexist in en but not here; without them, UI copy will fall back to English.i18n/locales/id-ID.json-321-335 (1)
321-335:⚠️ Potential issue | 🟡 MinorAdd missing
package.trendsstrings to prevent English fallback.
Please add translations forno_data,y_axis_label,legend_estimation, anditems.downloadsto match the new trends UI.i18n/locales/te-IN.json-250-263 (1)
250-263:⚠️ Potential issue | 🟡 MinorAdd missing
package.trendsstrings to avoid fallback.
Please add translations forno_data,y_axis_label,legend_estimation, anditems.downloadsso the trends UI doesn’t fall back to English.
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
187-192: Pin the action to a commit SHA for consistency and security.Line 189 uses
actions/upload-artifact@v4whilst other instances in this workflow (lines 97, 141) use the pinned SHA@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0. Using unpinned version tags poses a supply chain risk as tags can be moved to point to different commits.🔧 Suggested fix
- name: ⬆︎ Upload browser test report if: failure() - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: playwright-report path: playwright-report/
shuuji3
changed the title
| "trends": { | ||
| "items": {} | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
echo "=== Checking if i18n/locales directory exists ==="
ls -la i18n/locales/ 2>/dev/null | head -20 || echo "Directory not found, searching for locale files..."
echo ""
echo "=== Finding locale JSON files ==="
fd -t f '\.json$' i18n/locales 2>/dev/null | head -20Repository: npmx-dev/npmx.dev
Length of output: 1761
🏁 Script executed:
#!/bin/bash
echo "=== Looking for zh-CN.json ==="
fd -t f 'zh' i18n/locales
echo ""
echo "=== Checking en.json for package.trends structure ==="
cat i18n/locales/en.json | jq '.package.trends // "trends not found"'
echo ""
echo "=== Checking if zh-CN.json exists ==="
if [ -f i18n/locales/zh-CN.json ]; then
echo "zh-CN.json exists"
echo "=== Checking zh-CN.json package.trends ==="
cat i18n/locales/zh-CN.json | jq '.package.trends // "trends not found"'
else
echo "zh-CN.json does not exist in i18n/locales/"
echo "=== Listing all files in i18n/locales to verify ==="
ls -la i18n/locales/
fiRepository: npmx-dev/npmx.dev
Length of output: 892
Add missing translations to trends object; currently only empty items container exists.
The package.trends object in zh-CN.json is severely incomplete. The English locale (en.json) defines 16 translation keys (granularity labels, date range, loading states, axis labels, etc.) plus a "downloads" translation in items, but zh-CN.json provides none of these. Chinese (Simplified) users will encounter untranslated UI text for the trends feature.
All these keys must be translated and added to zh-CN.json: granularity, granularity_daily, granularity_weekly, granularity_monthly, granularity_yearly, start_date, end_date, loading, date_range, date_range_multiline, download_file, toggle_annotator, legend_estimation, no_data, y_axis_label, and items.downloads.
| "trends": { | ||
| "items": {} | ||
| }, |
There was a problem hiding this comment.
Populate package.trends keys instead of an empty stub.
Line 286–Line 288 introduce package.trends.items as an empty object. Since trend-related keys were moved out of package.downloads, this will surface missing zh-CN strings (or fallback text). Please migrate the previously removed zh-CN translations into package.trends.*.
| }, | ||
| "trends": {} | ||
| } |
There was a problem hiding this comment.
Add translations for compare.facets.trends or remove the block.
Line 923–Line 925 adds an empty compare.facets.trends object. If the UI expects compare.facets.trends.* keys, this will render missing labels in zh-CN. Please populate the translations or omit the block until it’s wired up.
|
I think I need to test this on my fork repository as |
2 participants
This is an experimental PR for better test reportings on GitHub Actions. I update
.github/workflows/ci.ymlto add test-summary/action (run always) and upload test result files as Artifacts when the test failed so that we can examine locally the HTML test reports by downloading the artifact.