search

pmem / pmemstream

Other
9 stars 13 forks source link

More tests #237

Closed lukaszstolarczuk closed 2 years ago

lukaszstolarczuk commented 2 years ago

This change is Reviewable

codecov-commenter commented 2 years ago

Codecov Report

Merging #237 (a80556e) into master (2b1a003) will increase coverage by 0.58%. The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #237      +/-   ##
==========================================
+ Coverage   88.90%   89.48%   +0.58%     
==========================================
  Files           9        9              
  Lines         721      742      +21     
  Branches      127      134       +7     
==========================================
+ Hits          641      664      +23     
+ Misses         48       47       -1     
+ Partials       32       31       -1
Flag Coverage Δ
tests_gcc_debug_cpp17 89.48% <ø> (+0.58%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pmemstream/src/span.c 83.87% <0.00%> (-2.62%) :arrow_down:
pmemstream/src/iterator.c 92.72% <0.00%> (-1.28%) :arrow_down:
pmemstream/src/singly_linked_list.h 100.00% <0.00%> (ø)
pmemstream/src/libpmemstream.c 88.16% <0.00%> (+0.88%) :arrow_up:
pmemstream/src/region.c 89.06% <0.00%> (+2.07%) :arrow_up:

:mega: Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

lukaszstolarczuk commented 2 years ago

Reviewed 10 of 11 files at r1, all commit messages. Reviewable status: 10 of 11 files reviewed, 2 unresolved discussions (waiting on @igchor and @lukaszstolarczuk)

_tests/api_c/async.c line 16 at r1 (raw file):_

        UT_FATAL("usage: %s file-name", argv[0]);
    }

empty?

_tests/api_c/entry_iterator.c line 172 at r1 (raw file):_

    /*
     * XXX: shouldn't this test also work if this re-allocation didn't happen?

Yes, we should probably extend pmemstream_entry_iterator_offset_is_inside_region to check if region is allocated and use this function in pmemstream_entry_iterator_is_valid

_tests/common/rapidcheck_helpers.hpp line 79 at r1 (raw file):_

        }

        UT_ASSERTeq(s.sut.persisted_timestamp(), total_entries_count);

How about calling timestamp_validate method here as well?

tests/unittest/append.cpp line 39 at r1 (raw file):

                     auto total_entries_count = data.size() + extra_data.size();
                     UT_ASSERTeq(total_entries_count, stream.sut.persisted_timestamp());

How about calling timestamp_validate method here as well?

Or perhaps, we should check it in stream dtor?

I can't see your issues on reviewable, all done.