llugin commented 7 years ago

Changes:

refactor of existing Sqlite scripts to make code reusable by LTP
elapsed time of single test case and whole execution now shown
changes in data structure for storing ran test results
handling for possible errors in utf-8 decoding of test outputs
tests that stopped failing compared to fails text file now listed (instead of 'some tests stopped failing')
LTP test scripts added
LTP integrated with docker

This change is

codecov-io commented 7 years ago

Codecov Report

Merging #324 into master will increase coverage by 1.7%. The diff coverage is n/a.

@@            Coverage Diff            @@
##           master     #324     +/-   ##
=========================================
+ Coverage   75.03%   76.73%   +1.7%     
=========================================
  Files          64       64             
  Lines        7922     7922             
  Branches     1589     1589             
=========================================
+ Hits         5944     6079    +135     
+ Misses       1479     1365    -114     
+ Partials      499      478     -21

Flag	Coverage Δ
#ltp_tests	`46.83% <ø> (?)`
#sqlite_tests	`45.53% <ø> (-0.02%)`	:arrow_down:
#tests_antool	`12.54% <ø> (ø)`	:arrow_up:
#tests_posix_multi_threaded	`25.96% <ø> (+0.1%)`	:arrow_up:
#tests_posix_single_threaded	`55.96% <ø> (-0.03%)`	:arrow_down:
#tests_preload	`46.16% <ø> (+0.05%)`	:arrow_up:

Impacted Files	Coverage Δ
src/libpmemfile-posix/inode.c	`88.5% <0%> (+0.34%)`	:arrow_up:
src/libpmemfile-posix/dir.c	`86.24% <0%> (+0.73%)`	:arrow_up:
src/libpmemfile-posix/read.c	`94.55% <0%> (+1.36%)`	:arrow_up:
src/libpmemfile/path_resolve.c	`89.88% <0%> (+1.78%)`	:arrow_up:
src/libpmemfile-posix/write.c	`89.61% <0%> (+2.59%)`	:arrow_up:
src/libpmemfile-posix/file.c	`73.74% <0%> (+2.79%)`	:arrow_up:
src/libpmemfile-posix/data.c	`93.07% <0%> (+3.46%)`	:arrow_up:
src/libpmemfile/preload.c	`54.12% <0%> (+4.91%)`	:arrow_up:
src/libpmemfile-posix/block_array.c	`97.5% <0%> (+5.83%)`	:arrow_up:
src/libpmemfile/libpmemfile-posix-wrappers.h	`68.6% <0%> (+15.11%)`	:arrow_up:
... and 2 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...afff1ba. Read the comment docs.

marcinslusarz commented 7 years ago

Reviewed 13 of 28 files at r3, 1 of 1 files at r4. Review status: 11 of 26 files reviewed at latest revision, 4 unresolved discussions.

utils/docker/external_tests/suite.py, line 59 at r3 (raw file):

        results = ['{0:>15} on {1} {2:>{3}.3f} [ms]'.
                   format(self.test_entry[config]['result'], config,
                          self.test_entry[config]['time'], 35 - len(config))

Where is 35 coming from?

utils/docker/external_tests/ltp/init.py, line 0 at r3 (raw file): Do we need empty file in repo?

utils/docker/images/Dockerfile.fedora-23, line 94 at r3 (raw file):

RUN gpasswd wheel -a $USER

USER $USER

Why do you switch users during installation?

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

#!/bin/bash -xe

Missing copyright and license.

Comments from Reviewable

GBuella commented 7 years ago

Review status: 11 of 26 files reviewed at latest revision, 4 unresolved discussions.

utils/docker/external_tests/ltp/init.py, line at r3 (raw file):

Previously, marcinslusarz (Marcin Ślusarz) wrote…

Do we need empty file in repo?

The copyright statement is missing from this file!

Comments from Reviewable

sarahjelinek commented 7 years ago

Review status: 11 of 26 files reviewed at latest revision, 7 unresolved discussions.

utils/docker/external_tests/run-build-suite.sh, line 75 at r4 (raw file):

  SUITE_UTILS_DIR=$TEST_UTILS_DIR/ltp
else
  echo "First argument doesn't match any of existing suites"\

s/any of existing suites/any existing test suites

utils/docker/external_tests/tester.py, line 94 at r4 (raw file):


        if set(self.failed_pf_only) == set(past_fails):
            print('No new fails.')

s/No new fails/No new failed tests

utils/docker/external_tests/tester.py, line 100 at r4 (raw file):

        if new_fails:
            exit_code = 1
            print('New fails introduced in this execution:')

s/New fails/New test failures

Comments from Reviewable

krzycz commented 7 years ago

Reviewed 11 of 22 files at r1, 28 of 28 files at r3, 1 of 1 files at r4. Review status: all files reviewed at latest revision, 7 unresolved discussions, some commit checks failed.

Comments from Reviewable

llugin commented 7 years ago

Review status: 13 of 26 files reviewed at latest revision, 7 unresolved discussions, some commit checks failed.

utils/docker/external_tests/run-build-suite.sh, line 75 at r4 (raw file):

Previously, sarahjelinek (Sarah Jelinek) wrote…

s/any of existing suites/any existing test suites

Done.

utils/docker/external_tests/suite.py, line 59 at r3 (raw file):

Previously, marcinslusarz (Marcin Ślusarz) wrote…

Where is 35 coming from?

assigned 35 to 'config_margin' variable

utils/docker/external_tests/tester.py, line 94 at r4 (raw file):

Previously, sarahjelinek (Sarah Jelinek) wrote…

s/No new fails/No new failed tests

Done.

utils/docker/external_tests/tester.py, line 100 at r4 (raw file):

Previously, sarahjelinek (Sarah Jelinek) wrote…

s/New fails/New test failures

Done.

utils/docker/external_tests/ltp/init.py, line at r3 (raw file):

Do we need empty file in repo? Yes.

utils/docker/images/Dockerfile.fedora-23, line 94 at r3 (raw file):

Previously, marcinslusarz (Marcin Ślusarz) wrote…

Why do you switch users during installation?

Fixed, ltp now installed by root.

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

Previously, marcinslusarz (Marcin Ślusarz) wrote…

Missing copyright and license.

Done.

Comments from Reviewable

krzycz commented 7 years ago

Reviewed 1 of 28 files at r3, 13 of 13 files at r5. Review status: all files reviewed at latest revision, 8 unresolved discussions, some commit checks failed.

utils/docker/external_tests/run-build-suite.sh, line 107 at r5 (raw file):


EXIT_CODE=`echo $?`

Is echo required here? EXIT_CODE=$?

Comments from Reviewable

llugin commented 7 years ago

Review status: 15 of 26 files reviewed at latest revision, 8 unresolved discussions.

utils/docker/external_tests/run-build-suite.sh, line 107 at r5 (raw file):

Previously, krzycz (Krzysztof Czurylo) wrote…

Is echo required here? `EXIT_CODE=$?`

Done.

Comments from Reviewable

krzycz commented 7 years ago

Reviewed 2 of 28 files at r3, 11 of 11 files at r6. Review status: all files reviewed at latest revision, 7 unresolved discussions.

Comments from Reviewable

marcinslusarz commented 7 years ago

Reviewed 2 of 28 files at r3, 7 of 11 files at r6. Review status: 25 of 26 files reviewed at latest revision, 6 unresolved discussions, some commit checks failed.

Comments from Reviewable

llugin commented 7 years ago

Review status: 14 of 26 files reviewed at latest revision, 5 unresolved discussions.

utils/docker/external_tests/run-build-suite.sh, line 110 at r6 (raw file):

Previously, marcinslusarz (Marcin Ślusarz) wrote…

Don't force your version when resolving conflicts!

Done.

Comments from Reviewable

GBuella commented 7 years ago

Reviewed 6 of 28 files at r3, 1 of 13 files at r5, 1 of 1 files at r7, 8 of 9 files at r8, 3 of 3 files at r9. Review status: 25 of 26 files reviewed at latest revision, 5 unresolved discussions.

utils/docker/external_tests/config.py, line 36 at r9 (raw file):


class Config:

I'm far from being a Python expert... what is the point of this class? Wouldn't basically an associative array be able to do the same job? Or, a function returning an associative array?

Comments from Reviewable

llugin commented 7 years ago

Review status: 25 of 26 files reviewed at latest revision, 5 unresolved discussions.

utils/docker/external_tests/config.py, line 36 at r9 (raw file):

Previously, GBuella (Gabor Buella) wrote…

I'm far from being a Python expert... what is the point of this class? Wouldn't basically an associative array be able to do the same job? Or, a function returning an associative array?

I found class properties to be useful here.

I need pf_env to be calculated dynamically on access. It has to contain all previously set env variables and since these env vars may be set by other parts of the python script (such thing happen with ltp script) it has to be up-to-date.
To prevent mountpoint dir to be changed outside initialization it has only getter.

Comments from Reviewable

sarahjelinek commented 7 years ago

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

Comments from Reviewable

krzycz commented 7 years ago

Reviewed 2 of 28 files at r3, 1 of 1 files at r7, 8 of 9 files at r8, 3 of 3 files at r10. Review status: all files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

Comments from Reviewable

GBuella commented 7 years ago

Reviewed 5 of 28 files at r3, 1 of 11 files at r6, 1 of 3 files at r10, 2 of 2 files at r11. Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

marcinslusarz commented 7 years ago

Reviewed 1 of 22 files at r2, 5 of 28 files at r3, 4 of 13 files at r5, 1 of 11 files at r6, 1 of 1 files at r7, 8 of 9 files at r8, 1 of 3 files at r10, 2 of 2 files at r11. Review status: all files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.

utils/docker/external_tests/run-build-suite.sh, line 46 at r11 (raw file):

if [[ -z "$1" ]]; then
  echo "ERROR: First argument has to contain a name of test suite"\
  "to be run (sqlite|ltp)."

S

Comments from Reviewable

marcinslusarz commented 7 years ago

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

utils/docker/external_tests/run-build-suite.sh, line 46 at r11 (raw file):

Previously, marcinslusarz (Marcin Ślusarz) wrote…

S

doesn't matter

Comments from Reviewable

krzycz commented 7 years ago

Reviewed 1 of 28 files at r3, 2 of 2 files at r11. Review status: :shipit: all files reviewed at latest revision, all discussions resolved, all commit checks successful.

Comments from Reviewable

marcinslusarz commented 7 years ago

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

Comments from Reviewable

GBuella commented 7 years ago

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

Comments from Reviewable

marcinslusarz commented 7 years ago

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

Comments from Reviewable

krzycz commented 7 years ago

Reviewed 1 of 2 files at r12. Review status: :shipit: all files reviewed at latest revision, all discussions resolved, all commit checks successful.

Comments from Reviewable

pmem / pmemfile

Docker: add LTP test scripts #324

Codecov Report