Skip to content
This repository has been archived by the owner on Apr 19, 2024. It is now read-only.

Add code coverage report #368

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

ivangergo3
Copy link

Added similar test-coverage report as we have in "cound-frontend"

So from now on, all tests will be executed when a pull request is being created.

And we will publish the results into google cloud, so we can check the current status here

Could you please check this change? And if you have any concerns let me know

@github-actions
Copy link

github-actions bot commented Feb 28, 2022

Coverage report

St.
Category Percentage Covered / Total
🔴 Statements 6.09% 781/12829
🔴 Branches 2.15% 155/7219
🔴 Functions 4.3% 108/2510
🔴 Lines 6.53% 769/11779

Test suite run success

67 tests passing in 16 suites.

Report generated by 🧪jest coverage report action from 9f8a6c7

@dimko dimko requested a review from a team February 28, 2022 13:00
tsconfig.json Outdated
"**/*.test.ts",
"**/*.test.tsx"
]
"include": ["./src/**/*", "src/vendor/dygraph-c91c859.min.js"],
Copy link

Choose a reason for hiding this comment

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

I don't see this dygraph js included in the previous version of the tsconfig.json, what is the reason for this being added now?


- uses: actions/setup-node@v2
with:
node-version: "12.22.0"
Copy link

@dimko dimko Mar 1, 2022

Choose a reason for hiding this comment

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

@netdata/cloud-fe should the version be that one? It's the same for every other test coverage PR, but previously it was set to node-version: '12.x'

- name: Run `npm install`
run: npm install
- name: Run `npm run lint`
run: npm run lint
Copy link

Choose a reason for hiding this comment

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

This file was deleted, but lint check wasn't moved to the other workflow

Copy link
Contributor

Choose a reason for hiding this comment

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

As @dimko mentioned, maybe the check for lint should be moved to the new file as well! 😄


on:
push:
branches: [master, develop]
Copy link
Member

Choose a reason for hiding this comment

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

Only master

@ivangergo3 ivangergo3 force-pushed the feature/add-coverage-report branch from 31d1ff7 to 572be63 Compare March 3, 2022 09:54
push:
branches: [master]
workflow_dispatch:
inputs:
Copy link
Contributor

Choose a reason for hiding this comment

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

It throws an error here for empty lines!
Maybe this should be removed! 😄

Copy link
Author

Choose a reason for hiding this comment

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

Good point, removed, update all pending workflows

Comment on lines 53 to 54
run: |
gsutil -h "Cache-Control: max-age=0, no-store" cp /tmp/coverage_report gs://${{ secrets.TEST_AUTOMATION_STORAGE_BUCKET }}/${{ github.event.repository.name }}/coverage_report
Copy link
Contributor

Choose a reason for hiding this comment

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

Also the checks throw an error here for too line size!

Copy link
Author

Choose a reason for hiding this comment

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

Updated in all pending workflows

@ivangergo3 ivangergo3 force-pushed the feature/add-coverage-report branch from 572be63 to 41eb232 Compare March 3, 2022 17:13
@ivangergo3
Copy link
Author

@jjtsou, it seems like it was a bad idea to remove the original testing pipeline. Should I put it back or should we run the new with all pull requests?

@ivangergo3
Copy link
Author

@novykh , it seems like it was a bad idea to remove the original testing pipeline. Should I put it back or should we run the new with all pull requests?

@novykh
Copy link
Member

novykh commented Mar 8, 2022

Run the new on pull_requests.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants