pmem / pmemfile

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

antool: add fault injection tests #347

Closed ldorau closed 7 years ago

ldorau commented 7 years ago

It will increase antool's code coverage to more than 98%.


This change is Reviewable

codecov[bot] commented 7 years ago

Codecov Report

Merging #347 into master will increase coverage by 2.72%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #347      +/-   ##
==========================================
+ Coverage   75.03%   77.75%   +2.72%     
==========================================
  Files          64       64              
  Lines        7922     7927       +5     
  Branches     1589     1592       +3     
==========================================
+ Hits         5944     6164     +220     
+ Misses       1479     1331     -148     
+ Partials      499      432      -67
Flag Coverage Δ
#sqlite_tests 45.45% <ø> (-0.1%) :arrow_down:
#tests_antool 15.32% <100%> (+2.78%) :arrow_up:
#tests_posix_multi_threaded 25.88% <ø> (+0.02%) :arrow_up:
#tests_posix_single_threaded 55.96% <ø> (-0.03%) :arrow_down:
#tests_preload 46.16% <ø> (+0.05%) :arrow_up:
Impacted Files Coverage Δ
src/tools/antool/syscall.py 97.12% <100%> (+15.85%) :arrow_up:
src/tools/antool/antool.py 100% <100%> (+25%) :arrow_up:
src/tools/antool/listsyscalls.py 99.25% <100%> (+15.72%) :arrow_up:
src/libpmemfile-posix/dir.c 85.01% <0%> (-0.5%) :arrow_down:
src/libpmemfile-posix/inode.c 88.5% <0%> (+0.34%) :arrow_up:
src/tools/antool/exceptions.py 100% <0%> (+16.66%) :arrow_up:
src/tools/antool/syscalltable.py 100% <0%> (+21.73%) :arrow_up:
... and 1 more

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...d42ab57. Read the comment docs.

marcinslusarz commented 7 years ago

tests/antool/test-fi.sh, line 290 at r1 (raw file):

Previously, ldorau (Lukasz Dorau) wrote…
Done

So now you are using -rf just with 2 commands. Not really an improvement.

I was thinking about something like that:

if [ -z "${DIR_NAME}" ]; then
  exit 1
fi
rm ${DIR_NAME}/*
rmdir ${DIR_NAME}

Comments from Reviewable

ldorau commented 7 years ago

Review status: 0 of 59 files reviewed at latest revision, 1 unresolved discussion.


tests/antool/test-fi.sh, line 290 at r1 (raw file):

Previously, marcinslusarz (Marcin Ślusarz) wrote…
So now you are using -rf just with 2 commands. Not really an improvement. I was thinking about something like that: ``` if [ -z "${DIR_NAME}" ]; then exit 1 fi rm ${DIR_NAME}/* rmdir ${DIR_NAME} ```

Done.


Comments from Reviewable

llugin commented 7 years ago

Review status: 0 of 59 files reviewed at latest revision, 2 unresolved discussions.


src/tools/antool/syscall.py, line 237 at r3 (raw file):

            self.log_print("corrupted entry packet information of syscall {0:s}:".format(self.name), debug)
            self.log_print("0x{0:016X} 0x{1:016X} {2:s} {3:s} [corrupted entry packet]"
                           .format(self.time_start, self.pid_tid, self.__str, self.name), debug)

I found 'str' name confusing, it's very similiar to built-in method str, while it's just a stripe :P. Maybe rename to stripe?


Comments from Reviewable

llugin commented 7 years ago

Review status: 0 of 59 files reviewed at latest revision, 2 unresolved discussions.


src/tools/antool/syscall.py, line 237 at r3 (raw file):

Previously, llugin wrote…
I found '__str' name confusing, it's very similiar to built-in method __str__, while it's just a stripe :P. Maybe rename to __stripe?

by str I meant str (apparently reviewable formats double underscores to bold)


Comments from Reviewable

ldorau commented 7 years ago

Review status: 0 of 59 files reviewed at latest revision, 2 unresolved discussions.


src/tools/antool/syscall.py, line 237 at r3 (raw file):

Previously, llugin wrote…
by __str__ I meant __ str __ (apparently reviewable formats double underscores to bold)

Done.


Comments from Reviewable

sarahjelinek commented 7 years ago

Reviewed 4 of 187 files at r1, 1 of 9 files at r2, 1 of 2 files at r4. Review status: 6 of 59 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.


tests/antool/output-err-fi-1.log.match, line 0 at r4 (raw file): Empty file?


Comments from Reviewable

ldorau commented 7 years ago

Review status: 6 of 59 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.


tests/antool/output-err-fi-1.log.match, line at r4 (raw file):

Previously, sarahjelinek (Sarah Jelinek) wrote…
Empty file?

Yes, the same like here: https://reviewable.io/reviews/pmem/pmemfile/341#-KtmoipPBqoMyQbWufpK

An empty match file means we check that there is no stderr (in this case) output for the test.


tests/antool/test-fi.sh, line 290 at r1 (raw file):

Previously, ldorau (Lukasz Dorau) wrote…
Done.

Ping


Comments from Reviewable

marcinslusarz commented 7 years ago

tests/antool/test-fi.sh, line 290 at r1 (raw file):

Previously, ldorau (Lukasz Dorau) wrote…
Ping

I still don't like it, but whatever.


Comments from Reviewable

sarahjelinek commented 7 years ago
:lgtm:

Review status: 4 of 77 files reviewed at latest revision, all discussions resolved, some commit checks failed.


Comments from Reviewable