Skip to content

Commit 19657ba

Browse files
lukebakkenlukebakken
committed
Convert unit_SUITE to property-based tests using PropEr
Replace the example-based assertions for `range_spec_to_location_number/2` with three PropEr properties, one per clause: - `prop_range_spec_to_location_number_suffix`: generates `FileSize >= 1` and `N` in `1..FileSize`, verifying `{FileSize - N, N}` - `prop_range_spec_to_location_number_prefix`: generates arbitrary positive pairs, verifying `{0, min(N, FileSize)}` - `prop_range_spec_to_location_number_byte_range`: generates `Start` in `0..FileSize-1` and `EndByte` in `Start..FileSize+100` or `undefined`, covering both explicit and open-ended ranges including end bytes beyond the file size as produced by real call sites `proper` is added to `TEST_DEPS`.
1 parent e93937b commit 19657ba

5 files changed

Lines changed: 100 additions & 27 deletions

File tree

‎Makefile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ PROJECT_DESCRIPTION = RabbitMQ S3 plugin
33
PROJECT_MOD = rabbitmq_stream_s3_app
44

55
DEPS = rabbit_common rabbit osiris khepri gun
6-
TEST_DEPS = rabbitmq_ct_helpers rabbitmq_ct_client_helpers rabbitmq_stream
6+
TEST_DEPS = rabbitmq_ct_helpers rabbitmq_ct_client_helpers rabbitmq_stream proper
77
LOCAL_DEPS = xmerl
88

99
DEP_EARLY_PLUGINS = rabbit_common/mk/rabbitmq-early-plugin.mk

‎src/rabbitmq_stream_s3_log_reader.erl‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,9 @@ resolve_remote_location(first, #{name := StreamId, shared := Shared}) ->
123123
resolve_remote_location(Offset, #{name := StreamId, shared := Shared}) when
124124
is_integer(Offset)
125125
->
126+
?LOG_DEBUG(?MODULE_STRING ":~ts/2 finding offset ~b for stream '~ts'", [
127+
?FUNCTION_NAME, Offset, StreamId
128+
]),
126129
FirstChunkId = osiris_log_shared:first_chunk_id(Shared),
127130
case Offset >= FirstChunkId of
128131
true ->
@@ -155,6 +158,9 @@ resolve_remote_location(Offset, #{name := StreamId, shared := Shared}) when
155158
end
156159
end;
157160
resolve_remote_location({timestamp, Ts} = Spec, #{name := StreamId}) ->
161+
?LOG_DEBUG(?MODULE_STRING ":~ts/2 finding timestamp ~b for stream '~ts'", [
162+
?FUNCTION_NAME, Ts, StreamId
163+
]),
158164
%% We can't cheaply query the first timestamp from `osiris_log_shared`.
159165
%% Instead try the remote tier first.
160166
case rabbitmq_stream_s3_server:get_manifest(StreamId) of

‎src/rabbitmq_stream_s3_server.erl‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -456,8 +456,10 @@ apply_effect(
456456
counters:put(Cnt, ?C_OSIRIS_LOG_FIRST_OFFSET, FstOff),
457457
counters:put(Cnt, ?C_OSIRIS_LOG_SEGMENTS, NumSegLeft);
458458
(Result) ->
459-
?LOG_DEBUG("trigger_retention EvalFun: unexpected result ~p for stream '~ts', skipping set_first_chunk_id",
460-
[Result, StreamId]),
459+
?LOG_DEBUG(
460+
"trigger_retention EvalFun: unexpected result ~p for stream '~ts', skipping set_first_chunk_id",
461+
[Result, StreamId]
462+
),
461463
ok
462464
end,
463465
Spec = [{'fun', local_retention_fun(StreamId)}],

‎test/s3_streams_SUITE.erl‎

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -363,9 +363,16 @@ read_from_remote_tier_by_offset(Config) ->
363363
%% confirming reads will go to the remote tier.
364364
?awaitMatch(
365365
[],
366-
rabbit_ct_broker_helpers:rpc(Config, 0, filelib, wildcard,
367-
[rabbit_ct_broker_helpers:get_node_config(Config, 0, data_dir) ++
368-
"/../stream/*/00000000000000000000.segment"]),
366+
rabbit_ct_broker_helpers:rpc(
367+
Config,
368+
0,
369+
filelib,
370+
wildcard,
371+
[
372+
rabbit_ct_broker_helpers:get_node_config(Config, 0, data_dir) ++
373+
"/../stream/*/00000000000000000000.segment"
374+
]
375+
),
369376
15000
370377
),
371378
RName = rabbit_misc:r(<<"/">>, queue, QName),
@@ -467,9 +474,16 @@ read_from_remote_tier_by_timestamp(Config) ->
467474
),
468475
?awaitMatch(
469476
[],
470-
rabbit_ct_broker_helpers:rpc(Config, 0, filelib, wildcard,
471-
[rabbit_ct_broker_helpers:get_node_config(Config, 0, data_dir) ++
472-
"/../stream/*/00000000000000000000.segment"]),
477+
rabbit_ct_broker_helpers:rpc(
478+
Config,
479+
0,
480+
filelib,
481+
wildcard,
482+
[
483+
rabbit_ct_broker_helpers:get_node_config(Config, 0, data_dir) ++
484+
"/../stream/*/00000000000000000000.segment"
485+
]
486+
),
473487
15000
474488
),
475489
RName = rabbit_misc:r(<<"/">>, queue, QName),
@@ -620,12 +634,9 @@ receive_deliver_loop(S, C0, SubId) ->
620634

621635
%% Extract the raw payload from an osiris chunk.
622636
extract_payload(Chunk) ->
623-
<<_Mag:4, _Ver:4, _Type:8, _NumEntries:16, _NumRecords:32,
624-
_Timestamp:64, _Epoch:64, _ChunkId:64,
625-
_Crc:32, _DataLength:32, _TrailerLength:32,
626-
BloomSize:16, _Reserved:16,
627-
_Bloom:BloomSize/binary,
628-
0:1, EntrySize:31, Entry:EntrySize/binary, _/binary>> = Chunk,
637+
<<_Mag:4, _Ver:4, _Type:8, _NumEntries:16, _NumRecords:32, _Timestamp:64, _Epoch:64,
638+
_ChunkId:64, _Crc:32, _DataLength:32, _TrailerLength:32, BloomSize:16, _Reserved:16,
639+
_Bloom:BloomSize/binary, 0:1, EntrySize:31, Entry:EntrySize/binary, _/binary>> = Chunk,
629640
Sections = amqp10_framing:decode_bin(Entry),
630641
#'v1_0.data'{content = Payload} = lists:keyfind('v1_0.data', 1, Sections),
631642
Payload.

‎test/unit_SUITE.erl‎

Lines changed: 66 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,13 @@
66
-compile([export_all, nowarn_export_all]).
77

88
-include_lib("common_test/include/ct.hrl").
9-
-include_lib("eunit/include/eunit.hrl").
9+
-include_lib("proper/include/proper.hrl").
1010

1111
all() ->
1212
[
13-
range_spec_to_location_number
13+
range_spec_to_location_number_suffix,
14+
range_spec_to_location_number_prefix,
15+
range_spec_to_location_number_byte_range
1416
].
1517

1618
init_per_suite(Config) ->
@@ -20,13 +22,65 @@ init_per_suite(Config) ->
2022
end_per_suite(Config) ->
2123
Config.
2224

23-
range_spec_to_location_number(_Config) ->
24-
%% Suffix range (negative integer): last N bytes.
25-
?assertEqual({900, 100}, rabbitmq_stream_s3_api_fs:range_spec_to_location_number(1000, -100)),
26-
%% Suffix range (positive integer): first N bytes, capped at file size.
27-
?assertEqual({0, 100}, rabbitmq_stream_s3_api_fs:range_spec_to_location_number(1000, 100)),
28-
?assertEqual({0, 50}, rabbitmq_stream_s3_api_fs:range_spec_to_location_number(50, 100)),
29-
%% Byte range with explicit end byte.
30-
?assertEqual({10, 91}, rabbitmq_stream_s3_api_fs:range_spec_to_location_number(1000, {10, 100})),
31-
%% Open-ended byte range: from start byte to end of file.
32-
?assertEqual({10, 990}, rabbitmq_stream_s3_api_fs:range_spec_to_location_number(1000, {10, undefined})).
25+
%% Suffix range (negative integer): last N bytes of a file.
26+
range_spec_to_location_number_suffix(_Config) ->
27+
rabbit_ct_proper_helpers:run_proper(
28+
fun prop_range_spec_to_location_number_suffix/0, [], 500
29+
).
30+
31+
prop_range_spec_to_location_number_suffix() ->
32+
?FORALL(
33+
{FileSize, N},
34+
?LET(FS, pos_integer(), {FS, integer(1, FS)}),
35+
begin
36+
{Loc, Num} = rabbitmq_stream_s3_api_fs:range_spec_to_location_number(FileSize, -N),
37+
(Loc =:= FileSize - N) andalso (Num =:= N)
38+
end
39+
).
40+
41+
%% Prefix range (positive integer): first N bytes, capped at file size.
42+
range_spec_to_location_number_prefix(_Config) ->
43+
rabbit_ct_proper_helpers:run_proper(
44+
fun prop_range_spec_to_location_number_prefix/0, [], 500
45+
).
46+
47+
prop_range_spec_to_location_number_prefix() ->
48+
?FORALL(
49+
{FileSize, N},
50+
{pos_integer(), pos_integer()},
51+
begin
52+
{Loc, Num} = rabbitmq_stream_s3_api_fs:range_spec_to_location_number(FileSize, N),
53+
(Loc =:= 0) andalso (Num =:= min(N, FileSize))
54+
end
55+
).
56+
57+
%% Byte range: explicit end byte or open-ended (undefined).
58+
range_spec_to_location_number_byte_range(_Config) ->
59+
rabbit_ct_proper_helpers:run_proper(
60+
fun prop_range_spec_to_location_number_byte_range/0, [], 500
61+
).
62+
63+
prop_range_spec_to_location_number_byte_range() ->
64+
?FORALL(
65+
{FileSize, Start, MaybeEnd},
66+
?LET(
67+
FS,
68+
pos_integer(),
69+
?LET(
70+
S,
71+
integer(0, FS - 1),
72+
{FS, S, oneof([integer(S, FS + 100), undefined])}
73+
)
74+
),
75+
begin
76+
{Loc, Num} = rabbitmq_stream_s3_api_fs:range_spec_to_location_number(
77+
FileSize, {Start, MaybeEnd}
78+
),
79+
Expected =
80+
case MaybeEnd of
81+
undefined -> FileSize - Start;
82+
End -> End - Start + 1
83+
end,
84+
(Loc =:= Start) andalso (Num =:= Expected)
85+
end
86+
).

0 commit comments

Comments
 (0)