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.

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

coverage: provide overall statistics about coverage #2184

Merged
merged 1 commit into from
Feb 15, 2023

Conversation

velias
Copy link
Contributor

@velias velias commented Sep 30, 2022

Description

Extension of the coverage module to provide overall statistics of the coverage.
Console output contains info like percentage of "Types covered", "Types fully covered" and "Fields fully covered" (fields stat contains also arguments).
JSON output contains more detailed output, like num of types, num of types covered, num of types fully covered, num of fields and num of fields covered (fields numbers contain also arguments).

Motivation is to have simple overall overview of the schema coverage. Also overall statistic number allows to create badge for source code repo of your project.

Fixes #2183

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

  • ./packages/core/__tests__/coverage/coverage.ts test has been extended to cover statistics.
  • ./example has been used to test coverage cli, example json oputput (coverage.json) is updated

Test Environment:

  • OS: ubuntu linux 22.04
  • NodeJS: v16.17.1

Checklist:

  • I have followed the CONTRIBUTING doc and the
    style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@velias velias force-pushed the master branch 3 times, most recently from 95912c9 to f8e98fe Compare October 3, 2022 11:29
@changeset-bot
Copy link

changeset-bot bot commented Jan 17, 2023

🦋 Changeset detected

Latest commit: 5c7550f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@graphql-inspector/coverage-command Patch
@graphql-inspector/cli Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@TuvalSimha TuvalSimha self-assigned this Feb 1, 2023
@TuvalSimha
Copy link
Collaborator

TuvalSimha commented Feb 1, 2023

Hey @velias :),

Thank you for the suggestion, I find it very useful!
Can you please fix/add some small things?

  • Add Changeset
  • Add one more unit test
  • Make sure the CI checks pass

Thank you!

@TuvalSimha TuvalSimha self-requested a review February 1, 2023 14:31
Copy link
Collaborator

@TuvalSimha TuvalSimha left a comment

Choose a reason for hiding this comment

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

  • Add Changeset
  • Add one more unit test
  • Make sure the CI checks pass

@velias
Copy link
Contributor Author

velias commented Feb 2, 2023

@TuvalSimha thanks, for sure I'll do my best to provide correct PR. I'm only not sure what do you mean by "Add one more unit test"? Which test, what exactly should be tested?

@TuvalSimha
Copy link
Collaborator

@TuvalSimha thanks, for sure I'll do my best to provide correct PR. I'm only not sure what do you mean by "Add one more unit test"? Which test, what exactly should be tested?

@velias Thank you!
Usually, we try to cover some edge cases for new features, this is not required, but if you have more unit test edge cases for that, it will be nice :)

@velias velias force-pushed the master branch 5 times, most recently from e50bd4f to 78c563f Compare February 3, 2023 09:56
@velias
Copy link
Contributor Author

velias commented Feb 3, 2023

@TuvalSimha OK, I implemented there changes:

  • rebased whole PR to latest master
  • added changeset
  • ran prettier so builds looks OK
  • added one unit test to core/coverage. I was not able to find any tests for command.
  • changed image with coverage command output for doc/website to show changed output

Hope this makes PR mergeable

@velias
Copy link
Contributor Author

velias commented Feb 5, 2023

@TuvalSimha I'm not sure why is pr / algolia / algolia-records-check (pull_request) failing. I'm even not sure what is this check doing. Any idea/help?
It may be related to coverage doc change, but there is not any reasonable error message sayin what is wrong, error is Error: Resource not accessible by integration

@TuvalSimha
Copy link
Collaborator

@TuvalSimha I'm not sure why is pr / algolia / algolia-records-check (pull_request) failing. I'm even not sure what is this check doing. Any idea/help? It may be related to coverage doc change, but there is not any reasonable error message sayin what is wrong, error is Error: Resource not accessible by integration

@velias Hey :)
Not sure why, I guess because it’s a fork?

@velias
Copy link
Contributor Author

velias commented Feb 5, 2023

@TuvalSimha OK, so does it mean my PR is OK ?

@TuvalSimha TuvalSimha self-requested a review February 15, 2023 15:25
@TuvalSimha TuvalSimha merged commit 1118f36 into kamilkisiela:master Feb 15, 2023
@TuvalSimha
Copy link
Collaborator

Hey @velias :)
Thank you very much! I found it very useful. Feel free to think about more statistics features for coverage and open new PRs.

@velias
Copy link
Contributor Author

velias commented Feb 16, 2023

@TuvalSimha thanks for merging.
I don't know about more stats yet, but I thought about producing badge image from overall stats. We use it in our CI/CD in gitlab repo. It is relatively easy to produce once stats are in coverage results, but maybe commandline command should have option to generate it OOTB. But it requires some dependency, eg. badge-maker. So let me know if its addition looks beneficial and you find it acceptable.

@velias
Copy link
Contributor Author

velias commented Feb 16, 2023

@TuvalSimha looks like core package was not released, so coverage command is broken now. Sorry, maybe I screwed changeset file (but I think I selected core during its creation). Any idea how to patch this?

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

Successfully merging this pull request may close these issues.

coverage: provide statistics about coverage
2 participants