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

Add test cases for xutils #37

Closed
wants to merge 2 commits into from
Closed

Conversation

tharun571
Copy link
Contributor

This pull request adds particular test cases for xeus-cpp.

should_print_version : Checks when the right or wrong argument is passed for the required syntax.

Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@tharun571 tharun571 marked this pull request as draft March 13, 2024 04:09
@codecov-commenter
Copy link

codecov-commenter commented Mar 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 46.98%. Comparing base (f80ca79) to head (ad18588).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main      #37   +/-   ##
=======================================
  Coverage   46.98%   46.98%           
=======================================
  Files          15       15           
  Lines         779      779           
=======================================
  Hits          366      366           
  Misses        413      413           

@anutosh491 anutosh491 marked this pull request as ready for review March 14, 2024 11:55
Copy link
Collaborator

@anutosh491 anutosh491 left a comment

Choose a reason for hiding this comment

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

I think this looks good. Thanks for the PR.
Maybe update the PR title , "Add test cases" seems vague (be more specific)
Also why did you convert this pr to draft, were we about to add something else too ?

@tharun571 tharun571 changed the title Add test cases Add test cases for xutils Mar 14, 2024
@tharun571
Copy link
Contributor Author

tharun571 commented Mar 14, 2024

@anutosh491 I thought of adding more test cases for other cpp files, but I will add and create a new pr after understanding more about the repo.

@vgvassilev
Copy link
Contributor

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 49.32%. Comparing base (1524afd) to head (36c9b12).
Report is 1 commits behind head on main.

Additional details and impacted files
☔ View full report in Codecov by Sentry. 📢 Have feedback on the report? Share it here.

The coverage not did not detect coverage increase. Is that expected?

@tharun571
Copy link
Contributor Author

tharun571 commented Mar 14, 2024

@anutosh491 @vgvassilev Code base has been rebased and pushed.

@tharun571 tharun571 requested a review from anutosh491 March 14, 2024 15:12
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@vgvassilev
Copy link
Contributor

Is that PR still related?

@tharun571
Copy link
Contributor Author

Is that PR still related?

Nop, the same features were added in #36 .

@vgvassilev
Copy link
Contributor

Can you close it then?

@tharun571 tharun571 closed this Mar 19, 2024
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.

4 participants