pmem / pmemfile

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

pjdfstest integration #335

Closed marcinslusarz closed 7 years ago

marcinslusarz commented 7 years ago

Unfortunately we can't enable it on Travis - process switching is too slow :(.


This change is Reviewable

krzycz commented 7 years ago

Reviewed 16 of 16 files at r1. Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.


utils/docker/pjdfstest/test.sh, line 77 at r1 (raw file):

mkfs.pmemfile ${PMEMFILE_POOL_FILE} ${PMEMFILE_POOL_SIZE}
pmemfile-mount ${PMEMFILE_POOL_FILE} ${PMEMFILE_MOUNT_POINT}
#export PMEMFILE_POOLS=${PMEMFILE_MOUNT_POINT}:${PMEMFILE_POOL_FILE}

?


Comments from Reviewable

marcinslusarz commented 7 years ago

utils/docker/pjdfstest/test.sh, line 77 at r1 (raw file):

Previously, krzycz (Krzysztof Czurylo) wrote…
?

Removed.


Comments from Reviewable

krzycz commented 7 years ago

Reviewed 2 of 2 files at r2. Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.


Comments from Reviewable

krzycz commented 7 years ago
:lgtm:

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


Comments from Reviewable

sarahjelinek commented 7 years ago

Review status: 9 of 15 files reviewed at latest revision, 1 unresolved discussion.


utils/docker/images/install-pjdfstest.sh, line 40 at r3 (raw file):


cd $HOME
curl -L -o pjdfstest.zip https://github.com/pjd/pjdfstest/archive/${commitid}.zip

What happens if the 'curl' command fails? Shouldn't you check for the exit code here? If there is a proxy failure or something else that gets in the way of downloading this image we should fail.


Comments from Reviewable

marcinslusarz commented 7 years ago

utils/docker/images/install-pjdfstest.sh, line 40 at r3 (raw file):

Previously, sarahjelinek (Sarah Jelinek) wrote…
What happens if the 'curl' command fails? Shouldn't you check for the exit code here? If there is a proxy failure or something else that gets in the way of downloading this image we should fail.

I curl fails the whole script fails. "-e" in the first line (bash -ex) of the script assures that.


Comments from Reviewable

sarahjelinek commented 7 years ago

Reviewed 4 of 16 files at r1, 1 of 2 files at r2. Review status: 9 of 15 files reviewed at latest revision, 1 unresolved discussion.


utils/docker/images/install-pjdfstest.sh, line 40 at r3 (raw file):

Previously, marcinslusarz (Marcin Ślusarz) wrote…
I curl fails the whole script fails. "-e" in the first line (bash -ex) of the script assures that.

Oh, missed that :-).


Comments from Reviewable

sarahjelinek commented 7 years ago
:lgtm:

Review status: 9 of 15 files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

codecov[bot] commented 7 years ago

Codecov Report

Merging #335 into master will increase coverage by 0.33%. The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #335      +/-   ##
========================================
+ Coverage   73.66%    74%   +0.33%     
========================================
  Files          64     64              
  Lines        7990   7998       +8     
  Branches     1613   1615       +2     
========================================
+ Hits         5886   5919      +33     
+ Misses       1588   1569      -19     
+ Partials      516    510       -6
Flag Coverage Δ
#sqlite_tests 45.68% <100%> (+0.06%) :arrow_up:
#tests_antool 12.2% <0%> (-0.02%) :arrow_down:
#tests_posix_multi_threaded 25.94% <0%> (+0.02%) :arrow_up:
#tests_posix_single_threaded 56.01% <0%> (-0.04%) :arrow_down:
#tests_preload 45.96% <100%> (+0.52%) :arrow_up:
Impacted Files Coverage Δ
src/libpmemfile/preload.c 49.11% <100%> (+1.99%) :arrow_up:
src/libpmemfile/path_resolve.c 88.09% <100%> (+0.51%) :arrow_up:
src/libpmemfile-posix/rename.c 92% <0%> (+0.4%) :arrow_up:
src/libpmemfile-posix/dir.c 85.5% <0%> (+0.73%) :arrow_up:

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 a3f66ea...97c5305. Read the comment docs.