-
Notifications
You must be signed in to change notification settings - Fork 14
feat: support URL sources upload to portal #1235
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
base: main
Are you sure you want to change the base?
Conversation
|
View your CI Pipeline Execution ↗ for commit 6fe2ae5
☁️ Nx Cloud last updated this comment at |
|
View your CI Pipeline Execution ↗ for commit 6c3dff0 ☁️ Nx Cloud last updated this comment at |
@code-pushup/ci
@code-pushup/cli
@code-pushup/core
@code-pushup/models
@code-pushup/create-cli
@code-pushup/nx-plugin
@code-pushup/axe-plugin
@code-pushup/coverage-plugin
@code-pushup/eslint-plugin
@code-pushup/js-packages-plugin
@code-pushup/jsdocs-plugin
@code-pushup/lighthouse-plugin
@code-pushup/typescript-plugin
@code-pushup/utils
commit: |
Code PushUp🤨 Code PushUp report has both improvements and regressions – compared current commit 8e17838 with previous commit 7b6e72c. 🕵️ See full comparison in Code PushUp portal 🔍 🏷️ Categories👍 3 groups improved, 👎 1 group regressed, 👍 5 audits improved, 👎 4 audits regressed, 16 audits changed without impacting score🗃️ Groups
30 other groups are unchanged. 🛡️ Audits
654 other audits are unchanged. |
Code PushUp🤨 Code PushUp report has both improvements and regressions – compared current commit 8e17838 with previous commit 7b6e72c. 💼 Project
|
| 🏷️ Category | ⭐ Previous score | ⭐ Current score | 🔄 Score change |
|---|---|---|---|
| Documentation | 🟡 70 | 🟡 70 | |
| Code coverage | 🟢 94 | 🟢 94 |
4 other categories are unchanged.
👍 2 groups improved, 👍 3 audits improved
🗃️ Groups
| 🔌 Plugin | 🗃️ Group | ⭐ Previous score | ⭐ Current score | 🔄 Score change |
|---|---|---|---|---|
| JSDocs coverage | Documentation coverage | 🟡 70 | 🟡 70 | |
| Code coverage | Code coverage metrics | 🟢 94 | 🟢 94 |
13 other groups are unchanged.
🛡️ Audits
| 🔌 Plugin | 🛡️ Audit | 📏 Previous value | 📏 Current value | 🔄 Value change |
|---|---|---|---|---|
| JSDocs coverage | Functions coverage | 🟨 12 undocumented functions | 🟨 12 undocumented functions | |
| Code coverage | Branch coverage | 🟩 90 % | 🟩 90.3 % | |
| Code coverage | Line coverage | 🟨 73 % | 🟨 73.2 % |
440 other audits are unchanged.
💼 Project core
🥳 Code PushUp report has improved.
🕵️ See full comparison in Code PushUp portal 🔍
| 🏷️ Category | ⭐ Previous score | ⭐ Current score | 🔄 Score change |
|---|---|---|---|
| Code coverage | 🟢 90 | 🟢 90 |
5 other categories are unchanged.
👍 1 group improved, 👍 2 audits improved
🗃️ Groups
| 🔌 Plugin | 🗃️ Group | ⭐ Previous score | ⭐ Current score | 🔄 Score change |
|---|---|---|---|---|
| Code coverage | Code coverage metrics | 🟢 90 | 🟢 90 |
14 other groups are unchanged.
🛡️ Audits
| 🔌 Plugin | 🛡️ Audit | 📏 Previous value | 📏 Current value | 🔄 Value change |
|---|---|---|---|---|
| Code coverage | Branch coverage | 🟨 83.2 % | 🟨 83.3 % | |
| Code coverage | Line coverage | 🟩 94.8 % | 🟩 94.8 % |
442 other audits are unchanged.
💼 Project ci
🤨 Code PushUp report has both improvements and regressions.
🕵️ See full comparison in Code PushUp portal 🔍
| 🏷️ Category | ⭐ Previous score | ⭐ Current score | 🔄 Score change |
|---|---|---|---|
| Code coverage | 🟢 92 | 🟢 92 |
5 other categories are unchanged.
👎 1 group regressed, 👍 1 audit improved, 👎 1 audit regressed
🗃️ Groups
| 🔌 Plugin | 🗃️ Group | ⭐ Previous score | ⭐ Current score | 🔄 Score change |
|---|---|---|---|---|
| Code coverage | Code coverage metrics | 🟢 92 | 🟢 92 |
14 other groups are unchanged.
🛡️ Audits
| 🔌 Plugin | 🛡️ Audit | 📏 Previous value | 📏 Current value | 🔄 Value change |
|---|---|---|---|---|
| Code coverage | Branch coverage | 🟨 83.6 % | 🟨 83.5 % | |
| Code coverage | Line coverage | 🟩 94.8 % | 🟩 94.8 % |
443 other audits are unchanged.
💼 Project utils
🥳 Code PushUp report has improved.
🕵️ See full comparison in Code PushUp portal 🔍
| 🏷️ Category | ⭐ Previous score | ⭐ Current score | 🔄 Score change |
|---|---|---|---|
| Documentation | 🟡 61 | 🟡 61 |
5 other categories are unchanged.
👍 1 group improved
🗃️ Groups
| 🔌 Plugin | 🗃️ Group | ⭐ Previous score | ⭐ Current score | 🔄 Score change |
|---|---|---|---|---|
| JSDocs coverage | Documentation coverage | 🟡 61 | 🟡 61 |
14 other groups are unchanged.
🛡️ Audits
All of 444 audits are unchanged.
10 other projects are unchanged.
| check => check.message, | ||
| ); | ||
| if (requiredMessages.length > 0) { | ||
| return requiredMessages.join('. '); |
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.
The unit tests don't cover a case where there are multiple messages in all or none. If it's a realistic case, I would test that checks the messages are joined correctly.
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.
It is a realistic case. Thanks for the pointer!
| if (requiredMessages.length > 0) { | ||
| return requiredMessages.join('. '); | ||
| } | ||
| return node.any[0]?.message ?? fallback; |
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.
Is it possible that using only the first message in any omits useful information?
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.
I don't think so. The any array contains alternative ways to fix the issue, and the first message is typically the most direct fix. The rest are just fallback options covered by the audit's docs link.
For example, an "Images must have alternative text" audit has only any failures:
- Element does not have an alt attribute
- aria-label attribute does not exist or is empty
- aria-labelledby attribute does not exist, references elements that do not exist or references elements that are empty
- Element has no title attribute
- Element's default semantics were not overridden with role="none" or role="presentation""
The first message is a compact, actionable fix, and the other alternatives are already mentioned in the audit docs.
Alternatively, a "Links must have discernible text" audit has both none and any failures:
none:
- Element is in tab order and does not have accessible text
any:
- Element does not have text that is visible to screen readers
- aria-label attribute does not exist or is empty
- aria-labelledby attribute does not exist, references elements that do not exist or references elements that are empty
- Element has no title attribute
The none message is the actual problem.
I also initially thought all the failure messages were useful, but they really aren't.
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.
Also, the Axe audit title and the docs URL usually provide all the necessary context to describe the failure. The issue source seems much more useful than the message. So, there is no need to clutter issue messages with full failure summaries.
Part of #677
Upgrades
@code-pushup/portal-clientto v0.17.0 and adds support for uploading and downloading issues with URL sources.Additionally, the Axe plugin issue messages are now built from the structured check results (
none/all/anyarrays) instead of parsing thefailureSummarystring. This produces cleaner messages and removes the selector prefix workaround that was needed before the portal supported URL sources.