Skip to content

Conversation

@aleksandar-apostolov
Copy link
Collaborator

Goal

The goal is to introduce tests for the un-tested StreamEndpointErrorInterceptor. Additional tests for the parsers are added.
The parser tests may seam meaningless, but by round-tripping each internal model through Moshi we prove the generated adapters still match the JSON written/read by the socket session. If someone tweaks a field name, changes a default, or removes a property, the test breaks instead of the change sneaking into production (which has happened before).
This is more so important because these models are not generated by any OpenAPI, but rather handwritten here.

Implementation

2 new test suits.

Testing

Tests should pass.

Checklist

  • Issue linked (if any)
  • Tests/docs updated
  • I have signed the Stream CLA (required for external contributors)

@aleksandar-apostolov aleksandar-apostolov added the pr:test Test-only changes label Sep 25, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Sep 25, 2025

PR checklist ✅

All required conditions are satisfied:

  • Title length is OK (or ignored by label).
  • At least one pr: label exists.
  • Sections ### Goal, ### Implementation, and ### Testing are filled.

🎉 Great job! This PR is ready for review.

Comment on lines 59 to 63
val json = adapter.toJson(request)
val decoded = adapter.fromJson(json)!!

// Then
assertEquals(request, decoded)
Copy link
Collaborator

Choose a reason for hiding this comment

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

If somebody renames/removes a property or adds a default, this test will still pass because the output (decoded) changes according to the input (request). E.g. if I rename language to lang, both will contain lang and still match.

Compilation breaks if changing the properties (without using android studio refactoring), but that's true regardless of the test doing serialization + deserialization.

Copy link
Collaborator Author

@aleksandar-apostolov aleksandar-apostolov Sep 25, 2025

Choose a reason for hiding this comment

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

This is correct. Let me update the tests. The goal is to not blindly change the models or the annotated fields.

@gpunto gpunto merged commit 916aec5 into develop Sep 25, 2025
5 checks passed
@gpunto gpunto deleted the error-interceptor-test branch September 25, 2025 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr:test Test-only changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants