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

chore: add test coverage commands #112

Merged
merged 4 commits into from
Jul 17, 2024
Merged

Conversation

fpgmaas
Copy link
Collaborator

@fpgmaas fpgmaas commented Jul 17, 2024

No description provided.

@fpgmaas fpgmaas requested a review from SemyonSinchenko July 17, 2024 05:39
@fpgmaas fpgmaas force-pushed the chore/test-coverage branch from ef6a25f to 8c48fba Compare July 17, 2024 06:32
Copy link
Collaborator

@SemyonSinchenko SemyonSinchenko left a comment

Choose a reason for hiding this comment

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

I do not fully understand the idea. Why do we need both html and xml? And why do we generate html by default. Why cannot we just use stdout? With a simple --cov=chispa it will print a nice and readable message with summary about coverage.

I would like to see the stdout message on a make test and html/xml reports should be generated by test-coverage or something like this.

Also, for example, I do not have an open command (xdg-open should be used instead). What do you think about removing open and generate only reports?

@fpgmaas
Copy link
Collaborator Author

fpgmaas commented Jul 17, 2024

I do not fully understand the idea. Why do we need both html and xml? And why do we generate html by default. Why cannot we just use stdout? With a simple --cov=chispa it will print a nice and readable message with summary about coverage.

I would like to see the stdout message on a make test and html/xml reports should be generated by test-coverage or something like this.

Also, for example, I do not have an open command (xdg-open should be used instead). What do you think about removing open and generate only reports?

@SemyonSinchenko Sorry, I could have elaborated a bit more in the PR description. The idea is to have:

  • A html report for local development; this allows you to easily see which lines from which file are covered or not covered. A lot more user friendly than xml
  • An xml report to add automated test coverage checking on PR's in a later stage, with codecov or a similar tool.

Hope this clarifies! I modified the commands and removed the open command for the HTML report.

Copy link
Collaborator

@SemyonSinchenko SemyonSinchenko left a comment

Choose a reason for hiding this comment

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

LGTM overall. I left one minor comment. Feel free to merge when you are done.

Makefile Outdated Show resolved Hide resolved
@fpgmaas fpgmaas force-pushed the chore/test-coverage branch from 5890b7a to 771b9fc Compare July 17, 2024 07:08
@fpgmaas fpgmaas merged commit 36b92f3 into MrPowers:main Jul 17, 2024
7 checks passed
@fpgmaas fpgmaas deleted the chore/test-coverage branch July 17, 2024 07:14
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.

2 participants