pmem / pmemfile

Userspace implementation of file APIs using persistent memory.
Other
34 stars 21 forks source link

antool: more minor fixes #353

Closed ldorau closed 7 years ago

ldorau commented 7 years ago

This change is Reviewable

krzycz commented 7 years ago

Reviewed 2 of 2 files at r1. Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

codecov[bot] commented 7 years ago

Codecov Report

Merging #353 into master will increase coverage by 0.01%. The diff coverage is 87.5%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #353      +/-   ##
==========================================
+ Coverage   75.05%   75.07%   +0.01%     
==========================================
  Files          64       64              
  Lines        7921     7915       -6     
  Branches     1589     1585       -4     
==========================================
- Hits         5945     5942       -3     
+ Misses       1479     1477       -2     
+ Partials      497      496       -1
Flag Coverage Δ
#sqlite_tests 45.88% <ø> (-0.04%) :arrow_down:
#tests_antool 12.56% <87.5%> (+0.02%) :arrow_up:
#tests_posix_multi_threaded 25.89% <ø> (-0.05%) :arrow_down:
#tests_posix_single_threaded 56% <ø> (+0.02%) :arrow_up:
#tests_preload 46.15% <ø> (-0.06%) :arrow_down:
Impacted Files Coverage Δ
src/tools/antool/listsyscalls.py 83.52% <100%> (ø) :arrow_up:
src/tools/antool/syscall.py 83.17% <85.71%> (+1.9%) :arrow_up:
src/libpmemfile-posix/dir.c 84.76% <0%> (-0.99%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 6d1e91e...b3912f1. Read the comment docs.

codecov[bot] commented 7 years ago

Codecov Report

Merging #353 into master will increase coverage by 0.06%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #353      +/-   ##
==========================================
+ Coverage   75.03%   75.09%   +0.06%     
==========================================
  Files          64       64              
  Lines        7922     7915       -7     
  Branches     1589     1584       -5     
==========================================
  Hits         5944     5944              
+ Misses       1479     1476       -3     
+ Partials      499      495       -4
Flag Coverage Δ
#sqlite_tests 45.54% <ø> (ø) :arrow_up:
#tests_antool 12.56% <100%> (+0.02%) :arrow_up:
#tests_posix_multi_threaded 25.92% <ø> (+0.06%) :arrow_up:
#tests_posix_single_threaded 55.99% <ø> (ø) :arrow_up:
#tests_preload 46.14% <ø> (+0.02%) :arrow_up:
Impacted Files Coverage Δ
src/tools/antool/listsyscalls.py 83.68% <100%> (+0.15%) :arrow_up:
src/tools/antool/syscall.py 83.17% <100%> (+1.9%) :arrow_up:
src/libpmemfile-posix/dir.c 85.25% <0%> (-0.25%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 829c0dd...b82bf7e. Read the comment docs.

sarahjelinek commented 7 years ago

Reviewed 1 of 2 files at r1. Review status: 1 of 2 files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.


src/tools/antool/listsyscalls.py, line 410 at r2 (raw file):

            # the SyS_flock syscall is not supported YET
            if syscall.name == "flock":
                return RESULT_UNSUPPORTED_YET

We don't have flock() listed as supported at all. Did this change?


src/tools/antool/listsyscalls.py, line 625 at r2 (raw file):


                if fd_out != -1:  # pragma: no cover
                    self.log_anls.warning("Unknown fd : {0:d}".format(fd_in))

I am a little confused by this 'no cover'. And it could be that it's because I don't completely understand the coverage utility but why would this piece of code never be executed and excluded from coverage?


src/tools/antool/listsyscalls.py, line 712 at r2 (raw file):

                            msg += " ({0:d})".format(fd)
                        else:  # pragma: no cover
                            msg += " (0x{0:016X})".format(fd)

Why no coverage here?


Comments from Reviewable

ldorau commented 7 years ago

Review status: all files reviewed at latest revision, 3 unresolved discussions.


src/tools/antool/listsyscalls.py, line 410 at r2 (raw file):

Previously, sarahjelinek (Sarah Jelinek) wrote…
We don't have flock() listed as supported at all. Did this change?

SyS_flock is listed as "unsupported yet" now: https://github.com/pmem/pmemfile/blob/master/STATUS.md

I cannot remember if its status was changed.


src/tools/antool/listsyscalls.py, line 625 at r2 (raw file):

Previously, sarahjelinek (Sarah Jelinek) wrote…
I am a little confused by this 'no cover'. And it could be that it's because I don't completely understand the coverage utility but why would this piece of code never be executed and excluded from coverage?

OK, it will be covered by a fault-injection test. I am removing this commit.


src/tools/antool/listsyscalls.py, line 712 at r2 (raw file):

Previously, sarahjelinek (Sarah Jelinek) wrote…
Why no coverage here?

OK, it will be covered by a fault-injection test. I am removing this commit.


Comments from Reviewable

krzycz commented 7 years ago

Reviewed 1 of 1 files at r3. Review status: all files reviewed at latest revision, 3 unresolved discussions.


Comments from Reviewable

krzycz commented 7 years ago

Reviewed 1 of 1 files at r4. Review status: all files reviewed at latest revision, 3 unresolved discussions.


Comments from Reviewable

sarahjelinek commented 7 years ago
:lgtm:

Review status: :shipit: all files reviewed at latest revision, all discussions resolved, all commit checks successful.


Comments from Reviewable