Skip to content

test(server): add API tests for message retrieval #2537

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.

Sign up for GitHub

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

Jump to bottom
Open
seokjin0414 wants to merge 4 commits into
base: master
Choose a base branch
from
Open

test(server): add API tests for message retrieval #2537

seokjin0414 wants to merge 4 commits into from
+1,028 −0

Conversation

@seokjin0414
Copy link

@seokjin0414 seokjin0414 commented Jan 5, 2026 •
edited
Loading

Summary

  • Add SDK API tests for message retrieval (offset + timestamp based)
  • 36 server configurations × 12 test cases = 432 parameter combinations covered
  • Server reuse optimization (36 servers instead of 432)

Test Coverage

  • 36 offset API
  • 36 timestamp API

Note: Parameter Values

SDK API tests use different parameter values than internal tests due to server validation requirements (segment_size must be multiple of 512, msgs_req_to_save min 32 and multiple of 32).
Internal tests bypass validation by creating SystemConfig directly. SDK API tests start real server processes, so validation is mandatory.
Closes #1657

@hubcio
Copy link
Contributor

hubcio commented Jan 8, 2026

hi, tests that you added are OK but that wasn't the goal.
please re-read the issue, mainly:
We should maintain the same test matrix parameters (message sizes, caching options, etc.)

you didn't do that. if you have any questions please ask on discord.

@seokjin0414
Copy link
Author

seokjin0414 commented Jan 8, 2026 •
edited
Loading

@hubcio I re-read the issue

  • Maintain the same test_matrix parameters from existing internal tests (get_by_offset.rs, get_by_timestamp.rs) - message size, batch size, segment size, cache indexes, etc.
  • Add new tests using real server + SDK API

Is this correct?

@hubcio
Copy link
Contributor

hubcio commented Jan 9, 2026 •
edited
Loading

@seokjin0414

The tests you added are functional and work correctly, but they don't address the core goal of #1657. Let me clarify what's needed.

The existing internal tests (get_by_offset.rs, get_by_timestamp.rs) use a test matrix with 432 parameter combinations:

  • Message sizes: 50B, 1KB, 20KB
  • Batch patterns: small/medium/large/very_large
  • Segment sizes: 10B, 200B, 10MB
  • Cache index configs: None, All, OpenSegment
  • Messages required to save: 1, 24, 1000, 10000

Your PR uses fixed hardcoded values (MESSAGES_COUNT = 2000, BATCH_LENGTH = 100) and doesn't vary any of these parameters. The whole point of the issue is to get the samecomprehensive coverage through the public API.

What's needed:

  1. Parameterized tests (or a loop-based approach to reduce server starts as mentioned in the issue)
  2. Variable message payload sizes matching the original tests
  3. Adjustable server configuration for segment size, cache indexes, flush thresholds
  4. Same batch patterns as the internal tests

Keep in mind that if you do it one-to-one, we'll spawn hundreds of iggy servers and the test time would be in hours, so you have to optimize it (reuse iggy-servers across multiple testcases).

Let me know if you have questions, happy to help on Discord.

@seokjin0414 seokjin0414 force-pushed the 1657-api-tests-message-retrieval branch 2 times, most recently from 5a239b9 to a7ac6b4 Compare January 10, 2026 17:27
@seokjin0414
Copy link
Author

seokjin0414 commented Jan 11, 2026 •
edited
Loading

@hubcio
I Refactored, Could you check if this direction is correct?

hubcio
hubcio requested changes Jan 15, 2026
View reviewed changes
@hubcio
Copy link
Contributor

hubcio commented Jan 16, 2026

Have you joined our discord?

@seokjin0414
Copy link
Author

seokjin0414 commented Jan 18, 2026

@hubcio Yes, I'm already on Discord.

@seokjin0414 seokjin0414 requested a review from hubcio January 18, 2026 05:07
@seokjin0414
test(server): add API tests for message retrieval
22b2daa 
Add 6 new API tests for message retrieval to consumer_timestamp_polling_scenario:
- test_offset_from_middle: poll from middle offset
- test_offset_beyond_end: poll beyond last offset returns empty
- test_timestamp_from_middle: poll from middle timestamp
- test_timestamp_future: poll with future timestamp returns empty
- test_message_content_verification: verify message ID and payload
- test_message_headers_verification: verify user headers

Closes apache#1657

Signed-off-by: seokjin0414 <[email protected]>
@seokjin0414
test(server): refactor message retrieval tests to use real Iggy API
3760c9b 
- Add get_messages_by_offset_api.rs for offset-based polling tests
- Add get_messages_by_timestamp_api.rs for timestamp-based polling tests
- Add message_retrieval.rs with test_matrix (72 test combinations)
- Remove consumer_timestamp_polling_scenario.rs (replaced by new tests)
- Cover 432 parameter combinations per polling strategy via server reuse

Closes apache#1657

Signed-off-by: seokjin0414 <[email protected]>
Signed-off-by: shin <[email protected]>
@seokjin0414
fix(test): restore consumer_timestamp_polling_scenario
9ea89b0 
Restore the scenario file that was incorrectly removed.
This test covers high-level consumer polling strategies
and is unrelated to the message retrieval API tests.

Signed-off-by: shin <[email protected]>
@seokjin0414 seokjin0414 force-pushed the 1657-api-tests-message-retrieval branch from c68aff0 to f3f8e9f Compare January 18, 2026 05:11
@seokjin0414 seokjin0414 force-pushed the 1657-api-tests-message-retrieval branch from f3f8e9f to 37204fe Compare January 18, 2026 05:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hubcio hubcio Awaiting requested review from hubcio

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Refactor message retrieval tests to use real Iggy API instead of internal components

2 participants

@seokjin0414 @hubcio