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: Increase Runtime coverage #5694

Merged
merged 6 commits into from
Sep 10, 2024
Merged

Conversation

MikaelMayer
Copy link
Member

This PR increases the Runtime coverage by adding more tests to cover previously uncovered lines.

By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.

@MikaelMayer MikaelMayer reopened this Aug 15, 2024
@MikaelMayer
Copy link
Member Author

Closing/Reopening PR because it seems that code coverage was below the previous value, which happens randomly. Given that this PR increases code coverage, this shouldn't happen, so running it again to have a second chance.
Anyway, even in that mode where some covered lines were missed, the PR is still at 87% lines covered (98681 / 113983) which might be 2% more than before.

@MikaelMayer
Copy link
Member Author

Reopening the PR made all tests to pass the first time, which resulted in a complete code coverage of 88%, which is indeed +2% of code coverage.

99992 / 113983

But the branch is out of date so I'm going to merge it. Anyway, this PR is good to merge.

@MikaelMayer
Copy link
Member Author

Should we now increase the coverage threshold to 87% ?

Copy link
Member

@keyboardDrummer keyboardDrummer left a comment

Choose a reason for hiding this comment

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

Some warnings are generated for the added code

@keyboardDrummer
Copy link
Member

Was some of the new code generated? Can you comment on why that was needed and how it was generated?

@MikaelMayer
Copy link
Member Author

Was some of the new code generated? Can you comment on why that was needed and how it was generated?

Good question. The code was generated by ChatGPT, and I reviewed it manually (like fixing the warnings you mentioned). Here is the conversation I had with it.
https://chatgpt.com/share/0070188d-21c6-4c5b-a825-ab2e03a6a74b

Why ChatGPT generation.

  • There is no macro system native to C# that I thought would be useful, like in Rust
  • It was extremely efficient to use ChatGPT for this task because it's straightforward (it took less than 2 minutes after writing the first three test cases to generate the remaining 13 others)
  • These coverage tests are a one-off and I don't expect them to change anytime soon.
  • If they need to change, any LLM should be able to update them
  • ChatGPT is one of the best chatbots in its free version.

I just realized we could simplify the assertions of the length in a loop to reduce the size of the file, which I just did.

@MikaelMayer MikaelMayer enabled auto-merge (squash) September 5, 2024 22:37
@keyboardDrummer
Copy link
Member

Was some of the new code generated? Can you comment on why that was needed and how it was generated?

Good question. The code was generated by ChatGPT, and I reviewed it manually (like fixing the warnings you mentioned). Here is the conversation I had with it. https://chatgpt.com/share/0070188d-21c6-4c5b-a825-ab2e03a6a74b

Why ChatGPT generation.

  • There is no macro system native to C# that I thought would be useful, like in Rust
  • It was extremely efficient to use ChatGPT for this task because it's straightforward (it took less than 2 minutes after writing the first three test cases to generate the remaining 13 others)
  • These coverage tests are a one-off and I don't expect them to change anytime soon.
  • If they need to change, any LLM should be able to update them
  • ChatGPT is one of the best chatbots in its free version.

I just realized we could simplify the assertions of the length in a loop to reduce the size of the file, which I just did.

Could you leave a comment describing the generation in the code? That way someone can regenerate it when necessary.

@MikaelMayer MikaelMayer merged commit 8388853 into master Sep 10, 2024
21 of 22 checks passed
@MikaelMayer MikaelMayer deleted the chore-increase-coverage branch September 10, 2024 14:25
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