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

Offering help with Unit Tests #1101

Closed
erlapso opened this issue Mar 11, 2025 · 4 comments
Closed

Offering help with Unit Tests #1101

erlapso opened this issue Mar 11, 2025 · 4 comments

Comments

@erlapso
Copy link

erlapso commented Mar 11, 2025

Hi everyone! I noticed a lot of Pull Requests, particularly in relation to new features and tests.

I'd like to help improve the test coverage using CodeBeaver, an AI agent that I built (it's free for open source!). To demonstrate what's possible, I've created a PR:

mpeg_ts: Rewrite and cleanup - Unit Tests

that adds comprehensive tests to the last Pull Request that was opened here: mpeg_ts: Rewrite and cleanup.

What the PR demonstrates:

  • Increases coverage by 21.74% for internal/hexpairwriter/hexpairwriter_test.go
  • Adds 1 new test in internal/hexpairwriter/hexpairwriter_test.go

On top of that: If you check the PR, you will see that CodeBeaver does not only aim for coverage - it also adds all the edge cases it can think of (we make it iterate 3 times on that alone).

How it works:

CodeBeaver analyzes your code changes in PRs:

  • Automatically generates and updates tests
  • Opens PRs with new/updated tests
  • Helps catch bugs before they reach production

Next steps

If this interests the maintainer team, I'm happy to:

  • Walk through how the tests were generated
  • Help set up automated test generation for future PRs (takes ~5 minutes)
  • Focus on specific areas you'd like to improve coverage for

Let me know what you think! Happy to adapt this approach based on the project's needs.


About CodeBeaver | GitHub

@wader
Copy link
Owner

wader commented Mar 11, 2025

Had a quick look. Not sure the new tests are related to mpeg_ts? fq's format tests usually use .fqtest files in combination with
some binary files etc encoding the format. Also looks like the files are not gofmt:ed correctly.

@wader
Copy link
Owner

wader commented Mar 15, 2025

Closing because lack of feedback. These kind of unsolicited semi-automated PRs/issues are not very useful if they are of this quality.

@wader wader closed this as completed Mar 15, 2025
@erlapso
Copy link
Author

erlapso commented Mar 20, 2025

Hey Wader, sorry for the late response. I was working on a structured reply to your point and it took me a little time. I appreciate the feedback and the time you took to review the PR so far. The TL;DR: is that we are working to improve the quality based on your feedback, aiming to release an update this week. In particular:

Not sure the new tests are related to mpeg_ts?

You are right, the way we were determining the files changed in the PR was a little confusing, so additional files were considered. We have since aligned it with how GitHub determines the PR's diff.

fq's format tests usually use .fqtest files in combination with
some binary files etc encoding the format.

I see your point. Would it be fair to define those tests as End2End tests rather than Unit Tests? The aim of the tests is to test the final functionality rather than a specific function. Would it be useful to have, on top of those, Unit Tests that test the single function?

Also looks like the files are not gofmt:ed correctly.
Great point. We are working on a functionality to apply gofmt to the code before committing.

If it's ok for you, I will keep you posted once we have the new release.

@wader
Copy link
Owner

wader commented Mar 28, 2025

Hey Wader, sorry for the late response. I was working on a structured reply to your point and it took me a little time. I appreciate the feedback and the time you took to review the PR so far. The TL;DR: is that we are working to improve the quality based on your feedback, aiming to release an update this week. In particular:

Not sure the new tests are related to mpeg_ts?

You are right, the way we were determining the files changed in the PR was a little confusing, so additional files were considered. We have since aligned it with how GitHub determines the PR's diff.

fq's format tests usually use .fqtest files in combination with
some binary files etc encoding the format.

I see your point. Would it be fair to define those tests as End2End tests rather than Unit Tests? The aim of the tests is to test the final functionality rather than a specific function. Would it be useful to have, on top of those, Unit Tests that test the single function?

Also looks like the files are not gofmt:ed correctly.
Great point. We are working on a functionality to apply gofmt to the code before committing.

If it's ok for you, I will keep you posted once we have the new release.

👍 read up a bit more about codebeaver, didn't know it was an open source project, sorry if i sounded a bit grumpy. But one feedback is that at least for fq i might have a bunch of open (some draft) PRs where i don't think tests are the main thing missing and also the current code might be more WIP just to test something for now so adding tests for it would not make much sense.

Yeap nearly all format decoder tests for fq are "end to end" by using an output format that is very verbose so more or less will assert that everything was decode correctly.

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

No branches or pull requests

2 participants