Failure of recent_aae_expiry test

[martinsumner]

recent_aae_expiry test failed:

%%% tictac_SUITE ==> {failed,{{badmatch,false},
         [{tictac_SUITE,recent_aae_expiry,1,
                        [{file,"/Users/martinsumner/dbroot/leveled/_build/test/lib/leveled/test/end_to_end/tictac_SUITE.erl"},
                         {line,885}]},
          {test_server,ts_tc,3,[{file,"test_server.erl"},{line,1533}]},
          {test_server,run_test_case_eval1,6,
                       [{file,"test_server.erl"},{line,1053}]},
          {test_server,run_test_case_eval,9,
                       [{file,"test_server.erl"},{line,985}]}]}}
Failed 1 tests. Passed 22 tests. 

I'm assuming this is intermittent at present. Re-testing to confirm. Speculative assumption is that it may be a timer sleep timing thing - @Licenser has noted problems elsewhere with this.

I don't think this test would have been run more than a couple of times before I added it.

[martinsumner]

Sleeping for an extra second appears to consistently resolve this.

The expiry is not required to be precise - so the test need not be too strict.

[martinsumner]

https://github.com/martinsumner/leveled/pull/84

[Licenser]

Mind if I bring up an alternative?

Sleeps are generally quite problematic in tests not only can they easily hide bugs they also more easily create false positives and all in all are not really deterministic.

I'm not nearly as familiar with the code as you but it seems like a (test time only) function like force_expiry would make this test deterministic and also quicker to run.

[martinsumner]

You're probably right. I suspect that this may be better be tested in a fundamentally different way (in fact it might already be covered well enough in eunit tests).

I'm going to sleep on it and think about it tomorrow.

It is good to have an alternative perspective on this. I've spent too long just looking at this on my own, and so I've got lazy in letting a few bad habits roll on.

[Licenser]

I've been there and done that more the once when you write the code it's super easy to get lost in it. I was thinking of making a PR doing some tests with EQC(mini). I've done something like that for a storage backend I've written: https://github.com/dalmatinerdb/mstore/blob/master/eqc/mstore_eqc.erl

It would not fully replace tests but it'd cover some different scenarios.

[russelldb]

Sounds great @Licenser. I've been promising EQC tests for ages, and not delivered yet. Did have a crack at getting https://github.com/basho/riak_kv/blob/develop/test/backend_eqc.erl working with leveled. Still on my machine right now.

[Licenser]

@russelldb !!! It's you :D I see some possible problems with that code so, it seems very kv specific on the one hand and secondary it uses the FSM which makes it a no-go for mini users :( and eqc-ci isn't really great either.

I wonder if for a start we could get away with a basic 'compare the data in leveled to a map/dict/pickyourdatatype' it'd sure not reach the same level of coverage as the backend but it can be implemented in a way that it'd run with mini which I think would be great for @martinsumner (unless he has an EQC license).

But then again the backend tests will be far more detailed then what I was thinking of and ultimately better. Perhaps the best is to do both? A set of simple cases that everyone can run and some super nice advanced tests that give a more complex set of coverage?

[Licenser]

PS: I think we should make this an own issue it's sure coplex enough