-
Notifications
You must be signed in to change notification settings - Fork 508
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: call setup before require_sds #6152
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 16 of 26 files at r1, all commit messages.
Reviewable status: 16 of 26 files reviewed, all discussions resolved (waiting on @grom72)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 10 of 26 files at r1.
Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @grom72)
src/test/pmempool_feature/TEST7
line 19 at r1 (raw file):
require_sds $PMEMPOOL$EXESUFFIX configure_valgrind pmemcheck force-enable $PMEMPOOL$EXESUFFIX
.
Suggestion:
configure_valgrind pmemcheck force-enable $PMEMPOOL$EXESUFFIX
setup
require_sds $PMEMPOOL$EXESUFFIX
src/test/pmempool_feature/TEST16
line 19 at r1 (raw file):
require_sds $PMEMPOOL$EXESUFFIX configure_valgrind force-disable
.
Suggestion:
configure_valgrind force-disable
setup
require_sds $PMEMPOOL$EXESUFFIX
src/test/pmempool_feature/TEST12
line 19 at r1 (raw file):
require_sds $PMEMPOOL$EXESUFFIX configure_valgrind force-disable
.
Suggestion:
configure_valgrind force-disable
setup
require_sds $PMEMPOOL$EXESUFFIX
src/test/pmempool_feature/TEST2
line 19 at r1 (raw file):
require_sds $PMEMPOOL$EXESUFFIX configure_valgrind force-disable
.
Suggestion:
configure_valgrind force-disable
setup
require_sds $PMEMPOOL$EXESUFFIX
src/test/pmempool_feature/TEST11
line 20 at r1 (raw file):
require_dax_devices 1 configure_valgrind force-disable
.
Suggestion:
require_dax_devices 1
configure_valgrind force-disable
setup
require_sds $PMEMPOOL$EXESUFFIX
src/test/pmempool_feature/TEST5
line 19 at r1 (raw file):
require_sds $PMEMPOOL$EXESUFFIX configure_valgrind memcheck force-enable $PMEMPOOL$EXESUFFIX
.
Suggestion:
configure_valgrind memcheck force-enable $PMEMPOOL$EXESUFFIX
setup
require_sds $PMEMPOOL$EXESUFFIX
src/test/pmempool_feature/TEST8
line 19 at r1 (raw file):
require_sds $PMEMPOOL$EXESUFFIX configure_valgrind pmemcheck force-enable $PMEMPOOL$EXESUFFIX
.
Suggestion:
configure_valgrind pmemcheck force-enable $PMEMPOOL$EXESUFFIX
setup
require_sds $PMEMPOOL$EXESUFFIX
src/test/pmempool_feature/TEST1
line 19 at r1 (raw file):
require_sds $PMEMPOOL$EXESUFFIX configure_valgrind force-disable
It is very confusing but since setup
assumes $CHECK_TYPE
is set and configure_valgrind
sets $CHECK_TYPE
I would argue configure_valgrind
ought to be called before setup
.
Suggestion:
configure_valgrind force-disable
setup
require_sds $PMEMPOOL$EXESUFFIX
src/test/pmempool_feature/TEST4
line 19 at r1 (raw file):
require_sds $PMEMPOOL$EXESUFFIX configure_valgrind memcheck force-enable $PMEMPOOL$EXESUFFIX
.
Suggestion:
configure_valgrind memcheck force-enable $PMEMPOOL$EXESUFFIX
setup
require_sds $PMEMPOOL$EXESUFFIX
src/test/pmempool_feature/TEST10
line 20 at r1 (raw file):
require_dax_devices 1 configure_valgrind force-disable
- Please see the argument about the
configure_valgrind
position. require_dax_devices
can be called before setup and has the potential to stop the test from even starting. I believe it should be called as soon as possible.
Suggestion:
require_dax_devices 1
configure_valgrind force-disable
setup
require_sds $PMEMPOOL$EXESUFFIX
This change is