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
cmake changes #10283
cmake changes #10283
Conversation
3787aec
to
014c443
Compare
|
retest this please. |
https://jenkins.ceph.com/job/ceph-pull-requests/9047/console |
fba9180
to
c4271e1
Compare
retest this please. |
@tchaikov the cmake part is above my paygrade but the tox/python part looks good to me :-) Reviewed-by: Loic Dachary <ldachary@redhat.com> |
thanks for your review! @dachary |
DISABLE_PIP_VERSION_CHECK= | ||
fi | ||
|
||
pip $DISABLE_PIP_VERSION_CHECK --version 1>&2 |
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.
please remove these lines, they are only for debugging purpose.
changelog
|
retest this please. |
changelog
|
i am getting this error in https://jenkins.ceph.com/job/ceph-pull-requests/9095/consoleFull @ErwanAliasr1 could this be related with some env problem in jenkins slave? |
retest this please. |
@tchaikov what was the purpose of changing the name of the "check" target to "test"? Is it to override the current "test" target that just runs ctest? |
this follows the pattern in ceph-disk. this enables us to run ceph-detect-init/run-tox.sh from the ${CMAKE_BINARY_DIRECTORY} as well. Signed-off-by: Kefu Chai <kchai@redhat.com>
@alimaredia that commit is not necessary at all. i just removed it. i am using "tests" for preparing the test dependency, which does not conflict with "test" or "check". |
$DRY_RUN make $BUILD_MAKEOPTS check || return 1 | ||
$DRY_RUN make $BUILD_MAKEOPTS all tests || return 1 | ||
$DRY_RUN ctest -L Racing -j1 --output-on-failure || return 1 | ||
$DRY_RUN ctest -LE Racing $CHECK_MAKEOPTS --output-on-failure || return 1 |
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.
@tchaikov why are there two runs of ctest here?
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.
please see commit message of 19a8b5c.
please note "make test" is used by cmake to run tests, so we cannot just repurpose it to *build* them. * AddCephTest.cmake: depends on "tests" * CMakeLists.txt: let "check" depend on "tests" * src/CMakeLists.txt: update the run-tox tests * run-make-check.sh: use "make tests" and "ctest" instead of "make check" * ceph-detect-init/CMakeLists.txt: let "tests" depend on "ceph-detect-init" * ceph-disk/CMakeLists.txt: let "tests" depend on "ceph-disk" Signed-off-by: Kefu Chai <kchai@redhat.com>
run-tox-ceph-disk and run-tox-ceph-detect-init are already added as test, so "ctest" and "make {test,check}" will run them without extra settings. Signed-off-by: Kefu Chai <kchai@redhat.com>
changelog
|
two tests timesout for unknown reasons, so label them with "Racing" and "LongRunning" labels. Signed-off-by: Kefu Chai <kchai@redhat.com>
this is a workaround of the timeout found in jenkins. currently three tests are found timeout, and they are labeld with "Racing" and "LongRunning". so, to workaround this issue, we run the tests in two phases: 1. run the racing tests with -j1 2. run the non-racing tests with -jN if we all all tests with -j1, the total test time is 2683.57 sec Signed-off-by: Kefu Chai <kchai@redhat.com>
as ceph_crypt.cc is using the symbols in it, and libcommon contains ceph_crypt.cc. Signed-off-by: Kefu Chai <kchai@redhat.com>
they are included by rgw_a as well. and ceph_rgw_jsonparser is linked against rgw_a. Signed-off-by: Kefu Chai <kchai@redhat.com>
on older versions of pip, this option is not supported, and --disable-pip-version-check is implied with --no-index. so no need to use them when --no-index is passed to pip. this partially reverts 395f2c5 Signed-off-by: Kefu Chai <kchai@redhat.com>
Signed-off-by: Kefu Chai <kchai@redhat.com>
quote from https://cmake.org/cmake/help/v3.0/prop_test/TIMEOUT.html?highlight=timeout > If it exceeds that the test process will be killed and ctest will move > to the next test. this helps us to identify test hang. Signed-off-by: Kefu Chai <kchai@redhat.com>
changelog
|
@tchaikov |
do you have the log and the backtrace? so we can look at it?
i spotted this fault before in the jenkins job, but it does not show up anymore. |
@cbodley is this good to merge? as "make check" is working as expected. |
I haven't tested it, but you addressed my concern about the 'check' dependencies. Thanks! |
@tchaikov I think this should get merged asap. I understand that this passes jenkins but you're not seeing the cephtool-test-mds.sh errors on your local machine like I am? |
not yet. if i had seen them, i'd fixed them already.
|
this changeset addresses following issues