test: No direct use of nose #45064

s-t-e-v-e-n-k · 2022-02-17T05:28:19Z

Checklist

Tracker (select at least one)
- References tracker ticket
- Very recent bug; references commit where it was introduced
- New feature (ticket optional)
- Doc update (no ticket needed)
- Code cleanup (no ticket needed)
Component impact
- Affects Dashboard, opened tracker ticket
- Affects Orchestrator, opened tracker ticket
- No impact that needs to be tracked
Documentation (select at least one)
- Updates relevant documentation
- No doc update is appropriate
Tests (select at least one)
- Includes unit test(s)
- Includes integration test(s)
- Includes bug reproducer
- No tests

With nose being firmly deprecated for quite some time, remove use of it from the test suite directly.

Fixes: https://tracker.ceph.com/issues/54252

tserong · 2022-02-21T04:21:09Z

jenkins test api

tserong · 2022-03-29T04:57:31Z

(I'm not 100% sure who's best to review this, so, @tchaikov, @cbodley, if you're not those people, are you able to recommend someone else?)

tchaikov

LGTM in general, will take a closer look tonight.

tchaikov · 2022-03-29T06:22:29Z

src/test/pybind/test_ceph_argparse.py


 # Local Variables:
 # compile-command: "cd ../../..; cmake --build build --target get_command_descriptions -j4 &&
 #  CEPH_BIN=build/bin \
-#  PYTHONPATH=src/pybind nosetests --stop \
-#  src/test/pybind/test_ceph_argparse.py:TestOSD.test_rm"
+#  PYTHONPATH=src/pybin python3 \


s/pybin/pybind/

cbodley · 2022-03-29T14:19:36Z

thanks @s-t-e-v-e-n-k, this is great. dropping nose has been on our "TODO someday maybe" list for a long time, so i'm thrilled that you're taking this on

these rgw multisite tests are complicated by the fact that they need to run in two different environments:

one from src/test/rgw/test_multi.py to deploy vstart clusters to run against, and
one from qa/tasks/rgw_multisite_tests.py to run under the rgw/multisite suite in teuthology

i see that you've covered test_multi.py and the stuff under rgw_multi/ already - have you managed to run that successfully to verify your changes there?

on the teuthology side, we also have some nose-specific logic in the rgw-multisite-tests task. i'm not good with python, so i had a lot of trouble getting this stuff to work. i think i tried using 'pytest' initially, with some guidance from @alfredodeza, but nose was the only thing i could actually get to load and run these test cases. this is further complicated by the need to run test cases from a different suite-branch to support upgrade tests

since this part only runs in teuthology, maybe it's fine to leave the leave the nose stuff in rgw_multisite_tests.py - i'll leave that up to. but we do need to verify that this invocation of nose.run() still works with your changes under src/test/rgw/rgw_multi/ (including the upgrade case that overrides the qa branch)

mcepl · 2022-03-29T14:23:17Z

i see that you've covered test_multi.py and the stuff under rgw_multi/ already - have you managed to run that successfully to verify your changes there?

We are Python people (actually, maintainers of Python packages in openSUSE) so most of what's going on in Ceph goes right above our head. Our primary goal is to eliminate nose from our distribution, so it is all we did.

cbodley · 2022-03-29T14:27:59Z

Our primary goal is to eliminate nose from our distribution, so it is all we did.

if your only goal is to drop the package dependency, then you probably don't need to touch these rgw tests at all - they don't get packaged anywhere

alfredodeza · 2022-03-29T14:28:36Z

My recommendation here would be to prioritize the work to remove nose in Ceph as a whole and work closely with the places there it is still needed to port over to Pytest. This is no longer a matter of preference, but Nose is not viable anymore as an unmaintained project. I anticipated that distributions dropping is going to be one of many problems to come if the Ceph teams decides to keep it around.

s-t-e-v-e-n-k · 2022-03-30T07:04:04Z

You're welcome @cbodley! I've rebased on current master, and corrected the typo that @tchaikov spotted. I have not successfully run the rgw_multi tests, I don't have a suitable test environment to hand, so some help there would be lovely, and I'd be delighted to help look at some test failures caused by my changes. I've also edited qa/tasks/rgw_multisite_tests.py to drop nose, but I'm not certain if my changes are correct there.

cbodley · 2022-03-30T14:25:57Z

qa/tasks/rgw_multisite_tests.py

-
-    return LogStream()
+        argv = [sys.executable, self.module_path + "../test_multi.py"] + extra_args
+        testsuite = subprocess.run(argv, check=True)


i don't think this can work. test_multi.py is creating its own ceph clusters with the vstart.sh script, but this teuthology script is meant to run tests against the clusters that teuthology itself set up

in qa/tasks/rgw_multisite.py, which runs just before this rgw_multisite_tests.py task, we do a lot of work to set up the python environment according to teuthology's deployment, and we pass this information into the python module here

so we need to run the test cases in that same environment - i don't think a subprocess can work here

cbodley · 2022-03-30T14:32:07Z

src/test/rgw/test_multi.py

-from rgw_multi.tests import *
-from rgw_multi.tests_es import *
-from rgw_multi.tests_ps import *
-from rgw_multi.tests_az import *


i tried running this modified test_multi.py by hand (according to docs in https://github.com/ceph/ceph/blob/master/src/test/rgw/test_multi.md), and it isn't finding any of the test cases to run. i believe it's because the tests are no longer imported into this module

~/ceph/build $ cat test_multi.conf [DEFAULT] num_zones = 2 num_ps_zones = 0 num_zonegroups = 1 gateways_per_zone = 1 ~/ceph/build $ RGW_MULTI_TEST_CONF=test_multi.conf MON=1 OSD=1 MGR=0 MDS=0 nosetests ../src/test/rgw/test_multi.py ---------------------------------------------------------------------- Ran 0 tests in 0.000s OK

cbodley · 2022-03-30T17:57:15Z

since these rgw tests are proving to be difficult, we might consider splitting that piece out to another PR? these other changes outside of rgw look a lot easier to verify and merge in the meantime

nose has been unmaintained for a long time, and to work on removing it as a dependency, re-work test_ceph_argparse to use bare unittest. Signed-off-by: Steve Kowalik <steven@wedontsleep.org>

nose has been unmaintained for a long time, and to work on removing it as a dependency, re-work test_ceph_daemon to use bare unittest. Signed-off-by: Steve Kowalik <steven@wedontsleep.org>

With the direct calls of nose methods removed, we no longer need to BuildRequire it. Fixes: https://tracker.ceph.com/issues/54252 Signed-off-by: Steve Kowalik <steven@wedontsleep.org>

s-t-e-v-e-n-k · 2022-04-04T01:58:11Z

What a great idea! I've split out the separate rgw test changes, and I'll create a new PR with that commit directly.

s-t-e-v-e-n-k · 2022-04-04T02:08:59Z

rgw has been split out to #45759

mcepl · 2022-05-11T10:17:46Z

@cbodley ping? I thought after #45064 (comment) and @s-t-e-v-e-n-k 's splitting the this PR, it could get merged. We really need to get rid of nose from our distro, it breaks some of our tools.

cbodley · 2022-05-11T13:49:02Z

@cbodley ping? I thought after #45064 (comment) and @s-t-e-v-e-n-k 's splitting the this PR, it could get merged. We really need to get rid of nose from our distro, it breaks some of our tools.

sorry, i'm only familiar with the rgw parts so assumed someone else would follow up on review here

but given @tchaikov's "LGTM in general", i'd be willing to approve this so long as it doesn't break anything in CI. i pushed this branch to ceph-ci to test the packaging at https://shaman.ceph.com/builds/ceph/pr-45064/

cbodley · 2022-05-11T13:49:08Z

jenkins test make check

cbodley · 2022-05-11T14:49:35Z

'make check' looks good 👍

135/277 Test 149: test_ceph_daemon.py ....................... Passed 0.30 sec
180/277 Test 150: test_ceph_argparse.py ..................... Passed 73.71 sec

waiting on packages to finish building

mcepl · 2022-05-12T06:10:28Z

Thank you!

github-actions bot added build/ops rgw tests labels Feb 17, 2022

tserong requested review from tchaikov and cbodley March 29, 2022 04:49

tchaikov reviewed Mar 29, 2022

View reviewed changes

s-t-e-v-e-n-k force-pushed the no-direct-use-of-nose branch from d0e9c7d to db65589 Compare March 30, 2022 07:00

cbodley reviewed Mar 30, 2022

View reviewed changes

s-t-e-v-e-n-k added 3 commits April 4, 2022 11:54

test/pybind/ceph_argparse: Switch to bare unittest

9f1c094

nose has been unmaintained for a long time, and to work on removing it as a dependency, re-work test_ceph_argparse to use bare unittest. Signed-off-by: Steve Kowalik <steven@wedontsleep.org>

test/pybind/ceph_daemon: Switch to bare unittest

34d8fc1

nose has been unmaintained for a long time, and to work on removing it as a dependency, re-work test_ceph_daemon to use bare unittest. Signed-off-by: Steve Kowalik <steven@wedontsleep.org>

ceph.spec.in,debian/control: Remove nose

47fba6f

With the direct calls of nose methods removed, we no longer need to BuildRequire it. Fixes: https://tracker.ceph.com/issues/54252 Signed-off-by: Steve Kowalik <steven@wedontsleep.org>

s-t-e-v-e-n-k force-pushed the no-direct-use-of-nose branch from db65589 to 47fba6f Compare April 4, 2022 01:56

cbodley removed the rgw label Apr 4, 2022

This was referenced Apr 13, 2022

test/rgw/test_multi: Switch to bare unittest #45759

Closed

cmake: resurrect mutex debugging in all Debug builds #45898

Merged

cbodley approved these changes May 11, 2022

View reviewed changes

cbodley merged commit f0e30f6 into ceph:master May 11, 2022

This was referenced May 13, 2022

quincy: test: No direct use of nose #46254

Merged

pacific: test: No direct use of nose #46255

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: No direct use of nose #45064

test: No direct use of nose #45064

s-t-e-v-e-n-k commented Feb 17, 2022 •

edited by tserong

tserong commented Feb 21, 2022

tserong commented Mar 29, 2022

tchaikov left a comment

tchaikov Mar 29, 2022

cbodley commented Mar 29, 2022

mcepl commented Mar 29, 2022

cbodley commented Mar 29, 2022

alfredodeza commented Mar 29, 2022

s-t-e-v-e-n-k commented Mar 30, 2022

cbodley Mar 30, 2022

cbodley Mar 30, 2022

cbodley commented Mar 30, 2022

s-t-e-v-e-n-k commented Apr 4, 2022

s-t-e-v-e-n-k commented Apr 4, 2022

mcepl commented May 11, 2022 •

edited

cbodley commented May 11, 2022

cbodley commented May 11, 2022

cbodley commented May 11, 2022 •

edited

mcepl commented May 12, 2022

test: No direct use of nose #45064

test: No direct use of nose #45064

Conversation

s-t-e-v-e-n-k commented Feb 17, 2022 • edited by tserong

Checklist

tserong commented Feb 21, 2022

tserong commented Mar 29, 2022

tchaikov left a comment

Choose a reason for hiding this comment

tchaikov Mar 29, 2022

Choose a reason for hiding this comment

cbodley commented Mar 29, 2022

mcepl commented Mar 29, 2022

cbodley commented Mar 29, 2022

alfredodeza commented Mar 29, 2022

s-t-e-v-e-n-k commented Mar 30, 2022

cbodley Mar 30, 2022

Choose a reason for hiding this comment

cbodley Mar 30, 2022

Choose a reason for hiding this comment

cbodley commented Mar 30, 2022

s-t-e-v-e-n-k commented Apr 4, 2022

s-t-e-v-e-n-k commented Apr 4, 2022

mcepl commented May 11, 2022 • edited

cbodley commented May 11, 2022

cbodley commented May 11, 2022

cbodley commented May 11, 2022 • edited

mcepl commented May 12, 2022

s-t-e-v-e-n-k commented Feb 17, 2022 •

edited by tserong

mcepl commented May 11, 2022 •

edited

cbodley commented May 11, 2022 •

edited