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 unit tests for org.asynchttpclient.netty.util.ByteBufUtils #1639

Merged

Conversation

EricHetti
Copy link
Contributor

I've analysed your codebase and noticed that org.asynchttpclient.netty.util.ByteBufUtils is not fully tested.
I've written some tests for the methods in this class with the help of Diffblue Cover.

Hopefully, these tests will help you detect any regressions caused by future code changes. If you would find it useful to have additional tests written for this repository, I would be more than happy to look at other classes that you consider important.

Copy link
Contributor

@slandelle slandelle left a comment

Choose a reason for hiding this comment

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

Thanks for contributing. Could you please address comments?

ByteBuf byteBuf2 = Unpooled.copiedBuffer(new byte[]{'o'});

ArrayAsserts.assertArrayEquals(new char[]{}, ByteBufUtils
.byteBuf2Chars(Charset.forName("ISO-8859-1"), byteBuf1));
Copy link
Contributor

Choose a reason for hiding this comment

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

Why use ISO-8859-1 and not US-ASCII?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As I was trying to cover the branch decodeChars(buf, charset) in the byteBuf2Chars(Charset charset, ByteBuf buf) method.

@EricHetti EricHetti force-pushed the add-diffblue-tests branch from fe3e2e0 to 8b10510 Compare June 11, 2019 10:21
Copy link
Contributor

@slandelle slandelle left a comment

Choose a reason for hiding this comment

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

Please address last comment and we'll be good to go. Thanks!

These tests were written using Diffblue Cover.
@EricHetti EricHetti force-pushed the add-diffblue-tests branch from 8b10510 to 5824732 Compare June 12, 2019 09:49
@EricHetti
Copy link
Contributor Author

Hi @slandelle just following up and wondering if you would like me to make any changes to the PR?

@slandelle slandelle merged commit febe50c into AsyncHttpClient:master Jun 25, 2019
@slandelle
Copy link
Contributor

Thanks!

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