Skip to content
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.

Sign up for GitHub

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

ci(unit-test): [alert,badge] add unit test actions #2682

Merged
merged 4 commits into from
Dec 25, 2024
Merged

ci(unit-test): [alert,badge] add unit test actions #2682

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6985dd2
ci(unit-test): [alert] add unit test actions
zzcr Dec 19, 2024
e4eb610
ci(unit-test): [alert] add unit test actions
zzcr Dec 19, 2024
099d06d
ci(unit-test): [alert] add unit test actions
zzcr Dec 19, 2024
3e2ef5d
ci(unit-test): [alert] add unit test actions
zzcr Dec 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 106 additions & 0 deletions .github/workflows/test-unit-pr.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
name: E2E Test PR
run-name: E2E Test PR--${{ github.event.pull_request.title }}
on:
pull_request:
types: [opened, reopened, synchronize, edited]

concurrency:
group: ${{ github.workflow }}-${{ github.event.number || github.sha }}
cancel-in-progress: true

jobs:
parse-components:
name: Parse Affected Components
runs-on: ubuntu-latest
outputs:
testComponents: ${{ steps.parseTitle.outputs.testComponents }}
steps:
- name: Parse Title
id: parseTitle
uses: actions/github-script@v6
with:
Comment on lines +20 to +21
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Update actions/github-script to latest version.

The current version @v6 is outdated. Update to the latest version to ensure security and feature improvements.

-        uses: actions/github-script@v6
+        uses: actions/github-script@v7
📝 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.

Suggested change
uses: actions/github-script@v6
with:
uses: actions/github-script@v7
with:
🧰 Tools
🪛 actionlint (1.7.4)

20-20: the runner of "actions/github-script@v6" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)

script: |
const prTitle = context.payload.pull_request.title
const regex = /\[(.*?)\]/
const matches = prTitle.match(regex)
if (matches && matches.length > 1 && matches[1]) {
let components = matches[1]
.split(',')
.map((c) => c.trim())
.filter((c) => /^[a-z\-\/]+$/.test(c))
.map((c) => `"\\/${c}\\/"`)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Strengthen component name validation.

The current regex /^[a-z\-\/]+$/ is too permissive and could allow invalid component names.

-                .filter((c) => /^[a-z\-\/]+$/.test(c))
+                .filter((c) => /^[a-z][a-z0-9\-\/]{2,}$/.test(c))
📝 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.

Suggested change
.filter((c) => /^[a-z\-\/]+$/.test(c))
.map((c) => `"\\/${c}\\/"`)
.filter((c) => /^[a-z][a-z0-9\-\/]{2,}$/.test(c))
.map((c) => `"\\/${c}\\/"`)

components = [...new Set(components)].slice(0, 3).join(' ')
core.setOutput('testComponents', components)
} else {
const warningString =`**[e2e-test-warn]**
The component to be tested is missing.

The title of the Pull request should look like "fix(vue-renderless): [action-menu, alert] fix xxx bug".

Please make sure you've read our [contributing guide](https://github.com/opentiny/tiny-vue/blob/dev/CONTRIBUTING.md)
`
core.setOutput('tip', warningString)
core.warning(warningString)
}
- name: generate user-tip.txt
if: ${{ steps.parseTitle.outputs.tip }}
run: |
cat << EOF > user-tip.txt
${{ steps.parseTitle.outputs.tip }}
EOF
- name: Upload User Tip
if: ${{ steps.parseTitle.outputs.tip }}
uses: actions/upload-artifact@v4
with:
name: user-tip
path: user-tip.txt
retention-days: 1
- name: Save PR number
if: ${{ steps.parseTitle.outputs.tip }}
run: echo ${{ github.event.number }} > ./pr-id.txt

- name: Upload PR number
if: ${{ steps.parseTitle.outputs.tip }}
uses: actions/upload-artifact@v4
with:
name: pr
path: ./pr-id.txt

pr-test:
if: ${{ needs.parse-components.outputs.testComponents }}
strategy:
matrix:
testcli: ${{ fromJson(needs.parse-components.outputs.testclis) }}

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix undefined matrix strategy configuration.

The matrix configuration references testclis which is not defined in the outputs of the parse-components job.

-    strategy:
-      matrix:
-        testcli: ${{ fromJson(needs.parse-components.outputs.testclis) }}

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 actionlint (1.7.4)

73-73: property "testclis" is not defined in object type {testcomponents: string}

(expression)

name: PR E2E Test
needs: parse-components
runs-on: ubuntu-latest
env:
TEST_COMPONENTS: ${{ needs.parse-components.outputs.testComponents }}
steps:
- uses: actions/checkout@v3
- name: Setup pnpm
uses: pnpm/action-setup@v2

- name: Setup node
uses: actions/setup-node@v3
with:
node-version: 20
- name: Get pnpm store directory
Comment on lines +76 to +84
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Update GitHub Actions to latest versions.

Several actions are using outdated versions:

-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
       - name: Setup pnpm
        uses: pnpm/action-setup@v2
       - name: Setup node
-        uses: actions/setup-node@v3
+        uses: actions/setup-node@v4
📝 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.

Suggested change
- uses: actions/checkout@v3
- name: Setup pnpm
uses: pnpm/action-setup@v2
- name: Setup node
uses: actions/setup-node@v3
with:
node-version: 20
- name: Get pnpm store directory
- uses: actions/checkout@v4
- name: Setup pnpm
uses: pnpm/action-setup@v2
- name: Setup node
uses: actions/setup-node@v4
with:
node-version: 20
- name: Get pnpm store directory
🧰 Tools
🪛 actionlint (1.7.4)

81-81: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)

86-86: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)

id: pnpm-cache
run: |
echo "pnpm_cache_dir=$(pnpm store path)" >> $GITHUB_OUTPUT

- uses: actions/cache@v3
name: Setup pnpm cache
with:
path: ${{ steps.pnpm-cache.outputs.pnpm_cache_dir }}
key: ${{ runner.os }}-pnpm-store-${{ hashFiles('**/pnpm-lock.yaml') }}
restore-keys: |
${{ runner.os }}-pnpm-store-

- name: Install dependencies
run: pnpm i --no-frozen-lockfile

Comment on lines +97 to +99
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Use frozen lockfile in CI environment.

For reproducible builds in CI, you should use --frozen-lockfile flag with pnpm install.

-        run: pnpm i --no-frozen-lockfile
+        run: pnpm i --frozen-lockfile
📝 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.

Suggested change
- name: Install dependencies
run: pnpm i --no-frozen-lockfile
- name: Install dependencies
run: pnpm i --frozen-lockfile

- name: E2E unit
run: pnpm test:unit3 ${{ env.TEST_COMPONENTS }}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add error handling and artifact upload for test results.

The test command should handle errors and upload test results as artifacts for debugging.

       - name: E2E unit
-        run: pnpm test:unit3 ${{ env.TEST_COMPONENTS }}
+        run: |
+          pnpm test:unit3 ${{ env.TEST_COMPONENTS }} || {
+            echo "Tests failed. Uploading test results..."
+            exit 1
+          }
+      - name: Upload test results
+        if: always()
+        uses: actions/upload-artifact@v4
+        with:
+          name: test-results
+          path: |
+            coverage/
+            test-results/
+          retention-days: 5
📝 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.

Suggested change
- name: E2E unit
run: pnpm test:unit3 ${{ env.TEST_COMPONENTS }}
- name: E2E unit
run: |
pnpm test:unit3 ${{ env.TEST_COMPONENTS }} || {
echo "Tests failed. Uploading test results..."
exit 1
}
- name: Upload test results
if: always()
uses: actions/upload-artifact@v4
with:
name: test-results
path: |
coverage/
test-results/
retention-days: 5

Loading