Skip to content

KAFKA-3631: Fix Struct.toString for nullable arrayOf#1279

Closed
granthenke wants to merge 3 commits into
apache:trunkfrom
granthenke:struct-fix
Closed

KAFKA-3631: Fix Struct.toString for nullable arrayOf#1279
granthenke wants to merge 3 commits into
apache:trunkfrom
granthenke:struct-fix

Conversation

@granthenke
Copy link
Copy Markdown
Member

No description provided.

assertFalse("Stuct string should not be empty.", stuctStr.isEmpty());
} catch (Throwable e) {
fail("Struct.toString throws an exception");
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is it really worth having this catch here? I think it's best to just let the exception propagate. Any method under test can throw an unknown exception after all.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I can remove it.

@Test
public void testToString() {
String stuctStr = this.struct.toString();
assertFalse("Stuct string should not be null.", stuctStr == null);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

assertNotNull?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Sure

@SinghAsDev
Copy link
Copy Markdown
Contributor

LGTM!

@asfgit asfgit closed this in 669be7f Apr 28, 2016
@ijuma
Copy link
Copy Markdown
Member

ijuma commented Apr 28, 2016

LGTM

gfodor pushed a commit to AltspaceVR/kafka that referenced this pull request Jun 3, 2016
Author: Grant Henke <granthenke@gmail.com>

Reviewers: Ismael Juma <ismael@juma.me.uk>

Closes apache#1279 from granthenke/struct-fix
mumrah pushed a commit to mumrah/kafka that referenced this pull request Aug 14, 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.

3 participants