iilyak commented on a change in pull request #3766:
URL: https://github.com/apache/couchdb/pull/3766#discussion_r731799976



##########
File path: src/smoosh/test/smoosh_priority_queue_tests.erl
##########
@@ -0,0 +1,119 @@
+-module(smoosh_priority_queue_tests).
+
+-include_lib("proper/include/proper.hrl").
+-include_lib("eunit/include/eunit.hrl").
+
+-define(PROP_PREFIX, "prop_").
+
+-define(CAPACITY, 3).
+
+-define(RANDOM_FILE, lists:flatten(io_lib:format("~p", [erlang:timestamp()]))).
+
+%% ==========
+%% Tests
+%% ----------
+
+%% define all tests to be able to run them individually
+prop_inverse_test_() -> test_property(prop_inverse).
+
+no_halt_on_corrupted_file_test() ->
+    Name = ?RANDOM_FILE,
+    Q0 = smoosh_priority_queue:new(Name),
+    Q = smoosh_priority_queue:open(Q0),
+    FilePath = smoosh_priority_queue:file_name(Q),
+    ok = file:write_file(FilePath, <<"garbage">>),
+    ?assertEqual(Q, smoosh_priority_queue:open(Q0)),
+    ok.
+
+no_halt_on_missing_file_test() ->
+    Name = ?RANDOM_FILE,
+    Q0 = smoosh_priority_queue:new(Name),
+    Q = smoosh_priority_queue:open(Q0),
+    FilePath = smoosh_priority_queue:file_name(Q),
+    ok = file:delete(FilePath),
+    ?assertEqual(Q, smoosh_priority_queue:open(Q0)),
+    ok.
+
+%% ==========
+%% Properties
+%% ----------
+
+prop_inverse() ->
+    ?FORALL(Q, queue(),
+        begin
+            List = smoosh_priority_queue:to_list(Q),
+            equal(Q, smoosh_priority_queue:from_list(List, Q))
+        end).
+
+%% ==========
+%% Generators
+%% ----------
+
+key() -> term().

Review comment:
       This is not technically correct. When you use 
`erlang:binary_to_term(Binary, [safe]);` you assume there are no things like 
atoms, pids, refs and funs. 
   
   I can see few solutions to it. 
   
   1. write `safe_term()` definition (modified after 
https://github.com/proper-testing/proper/blob/c6adaaa0dd5ac0a485e0cad16a73849a8612fbea/src/proper_types.erl#L1136):
   
   ```
   -spec safe_term() -> proper_types:type().
   safe_term() ->
       SafeTypes = 
[integer(),float(),bitstring(),?LAZY(loose_tuple(safe_term())),
                ?LAZY(list(safe_term())), ?LAZY(map(safe_term(), safe_term()))],
       proper_types:subtype([{generator, fun proper_gen:any_gen/1}], 
union(SafeTypes)).
   ```
   
   2. write exact types as used by smoosh `key() -> 
proper_types:oneof([proper_types:binary(), {proper_types:binary(), 
proper_types:binary()}]).`. I think `value` would be the same. 
   
   ```
   26> proper_gen:pick(proper_types:oneof([proper_types:binary(), 
{proper_types:binary(), proper_types:binary()}])).
   {ok,<<229,78,110,155,219,79,168>>}
   27> proper_gen:pick(proper_types:oneof([proper_types:binary(), 
{proper_types:binary(), proper_types:binary()}])).
   {ok,{<<"ôì">>,<<"ù">>}}
   ```
   
   3. Just add a comment saying something like: "The `atom` values produced by 
proper are known to the beam therefore it satisfy restrictions imposed by 
`erlang:binary_to_term(Binary, [safe]);``"
   
   #3 is easiest, while #2 would make testing faster. I. don't have a 
preference in this case.   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to