Skip to content

Add boundary tests for JsonTreeWriter.endArray() error handling#2988

Merged
eamonnmcmanus merged 3 commits intogoogle:mainfrom
lmj798:main
Mar 9, 2026
Merged

Add boundary tests for JsonTreeWriter.endArray() error handling#2988
eamonnmcmanus merged 3 commits intogoogle:mainfrom
lmj798:main

Conversation

@lmj798
Copy link
Copy Markdown
Contributor

@lmj798 lmj798 commented Mar 5, 2026

Summary

Add 3 boundary tests for JsonTreeWriter.endArray() to verify proper exception throwing when called in invalid states:

  1. testEndArrayOnEmptyStackThrows - Verifies IllegalStateException when calling endArray() on empty stack
  2. testEndArrayWithPendingNameThrows - Verifies IllegalStateException when a pending name exists (object not closed)
  3. testEndArrayWhenStackTopIsNotArrayThrows - Verifies IllegalStateException when stack top is an object, not an array

Why

Current tests cover:

  • Normal array/object begin/end operations
  • Close after write
  • Premature close
  • Null handling
  • NaN/Infinity handling
    However, they don't explicitly test endArray() error conditions for invalid state detection. These boundary cases ensure proper state validation and prevent regressions from state management changes.

Verification

mvn -Dtest=JsonTreeWriterTest test
Result: Tests run: 25, Failures: 0, Errors: 0, Skipped: 0 - BUILD SUCCESS

@google-cla
Copy link
Copy Markdown

google-cla Bot commented Mar 5, 2026

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Copy link
Copy Markdown
Member

@eamonnmcmanus eamonnmcmanus left a comment

Choose a reason for hiding this comment

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

This seems like a reasonable change. Just one question.

Comment thread gson/src/test/java/com/google/gson/internal/bind/JsonTreeWriterTest.java Outdated
@Marcono1234
Copy link
Copy Markdown
Contributor

Could you please move your last commit (073d0f5) to a separate PR? It is unrelated to JsonTreeWriter.endArray().

@lmj798
Copy link
Copy Markdown
Contributor Author

lmj798 commented Mar 6, 2026

Could you please move your last commit (073d0f5) to a separate PR? It is unrelated to JsonTreeWriter.endArray().

Sorry about that, and thank you for catching it.
You’re absolutely right that commit 073d0f5 was unrelated. I’ve moved it to a separate PR and force-pushed this branch, so this PR now only contains the JsonTreeWriter.endArray() changes.

@lmj798 lmj798 requested a review from eamonnmcmanus March 9, 2026 12:46
Copy link
Copy Markdown
Member

@eamonnmcmanus eamonnmcmanus left a comment

Choose a reason for hiding this comment

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

Thanks, it's good to check these extra cases.

@eamonnmcmanus eamonnmcmanus merged commit 36cacf3 into google:main Mar 9, 2026
16 checks passed
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