Page MenuHomeSoftware Heritage

Add tests using Celery.
ClosedPublic

Authored by vlorentz on Oct 11 2018, 5:28 PM.

Diff Detail

Repository
rDCIDX Metadata indexer
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

any idea why?

No clue.
On my part, it hangs so far...

 tony  (e) .venv   arcpatch-D518  …  swh  swh-environment  swh-indexer  pytest --maxfail=1 --capture=no
===================================================================================== test session starts =====================================================================================
platform linux -- Python 3.6.6, pytest-3.8.2, py-1.6.0, pluggy-0.7.1
hypothesis profile 'default' -> database=DirectoryBasedExampleDatabase('/home/tony/work/inria/repo/swh/swh-environment/swh-indexer/.hypothesis/examples')
rootdir: /home/tony/work/inria/repo/swh/swh-environment/swh-indexer, inifile:
plugins: requests-mock-1.5.2, hypothesis-3.74.0, celery-4.2.1
collected 86 items

swh/indexer/tests/test_language.py ...
swh/indexer/tests/test_metadata.py ......
swh/indexer/tests/test_mimetype.py ....
swh/indexer/tests/test_orchestrator.py ^C <- killed it

====================================================================================== warnings summary =======================================================================================
swh/indexer/tests/test_language.py
64 ↗(On Diff #1617)

duplicate

any idea why?

No clue.

I guess it is the OrchestratorTest that provokes this error, since this won't use the celery setup fixture, so the result_backend celery configuration is unset at this point.

On my part, it hangs so far...

 tony  (e) .venv   arcpatch-D518  …  swh  swh-environment  swh-indexer  pytest --maxfail=1 --capture=no
===================================================================================== test session starts =====================================================================================
platform linux -- Python 3.6.6, pytest-3.8.2, py-1.6.0, pluggy-0.7.1
hypothesis profile 'default' -> database=DirectoryBasedExampleDatabase('/home/tony/work/inria/repo/swh/swh-environment/swh-indexer/.hypothesis/examples')
rootdir: /home/tony/work/inria/repo/swh/swh-environment/swh-indexer, inifile:
plugins: requests-mock-1.5.2, hypothesis-3.74.0, celery-4.2.1
collected 86 items

swh/indexer/tests/test_language.py ...
swh/indexer/tests/test_metadata.py ......
swh/indexer/tests/test_mimetype.py ....
swh/indexer/tests/test_orchestrator.py ^C <- killed it

====================================================================================== warnings summary =======================================================================================

because this TestCase-based test need something like D519

swh/indexer/tests/test_orchestrator.py
123

I don't get it, why is the mocked method signature different than BaseOrchestratorIndexer._run_tasks() ?

  • Fix names.
  • Use the CeleryTestFixture from D519 instead of pytest fixtures.
vlorentz retitled this revision from [WIP] Add tests using Celery. to Add tests using Celery..Oct 15 2018, 2:34 PM
vlorentz edited the summary of this revision. (Show Details)
vlorentz added a reviewer: douardda.

The celery.contrib.testing module only was introduced in Celery 4.0, which Debian stable doesn't have. This means we get to backport celery and its dependency stack, yay.

I could copy-paste the code from Celery

  • No need to expose current_app to the actual test code.
douardda added inline comments.
swh/indexer/tests/__init__.py
3–4

I've just updated D531 so you should be able to use the (now very simple) CeleryTestFixture as base for OrchestratorTest

This revision now requires changes to proceed.Oct 19 2018, 10:50 AM
  • Use CeleryTestFixture to set os.environ.

Has this diff been rebased on latest swh-env's tests adaptation?

After applying patch, I have errors on tests about TEST_STORAGE_DB_NAME.
I remember having seen that variable being renamed to TEST_DB_NAME.

After checking the current swh-env, it's indeed that name which is used everywhere but the indexer.

swh/indexer/tests/__init__.py
28

I do not get all that's going on in that module.

As far as i understood the os.environ setup are needed to satisfy celery.
What about the rest (start_worker_thread, @shared_task [1] [2])?

Can you please add some comments to explain why all this is needed?

[1] The opened PR you mentioned is not merged yet and someone asked you a question there ;)

[2] Also, does that means we depend on celery 4 for the tests?
I'm not sure how to deal with this from the deployment point of view... (debian stable's celery version is 3.1.23 for now).

vlorentz added inline comments.
swh/indexer/tests/__init__.py
28

As far as i understood the os.environ setup are needed to satisfy celery.

Yes

What about the rest (start_worker_thread, @shared_task [1] [2])?

start_worker_thread is used by tests that trigger Celery tasks. I can used @task instead of @shared_task, that shouldn't be an issue.

Can you please add some comments to explain why all this is needed?

I don't know where the DATA_DIR comes from... Probably an issue when I rebased, I'll remove it.

[2] Also, does that means we depend on celery 4 for the tests?

I guess :/

  • Remove useless constant.

This sounds fine...

Except for the new library version which causes debian build package breakage...

irc discussion:

14:20 <+ardumont> still, that might (don't remember if they are run or not) break during debian packaging time
14:20 <vlorentz> it will

Indeed, it breaks:

$ cd swh-environment
$ bin/make-package -d stable -b swh-indexer
======================================================================
ERROR: Failure: ImportError (No module named 'celery.contrib.testing')
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/nose/failure.py", line 39, in runTest
    raise self.exc_val.with_traceback(self.tb)
  File "/usr/lib/python3/dist-packages/nose/loader.py", line 418, in loadTestsFromName
    addr.filename, addr.module)
  File "/usr/lib/python3/dist-packages/nose/importer.py", line 47, in importFromPath
    return self.importFromDir(dir_path, fqname)
  File "/usr/lib/python3/dist-packages/nose/importer.py", line 94, in importFromDir
    mod = load_module(part_fqname, fh, filename, desc)
  File "/usr/lib/python3.5/imp.py", line 244, in load_module
    return load_package(name, filename)
  File "/usr/lib/python3.5/imp.py", line 216, in load_package
    return _load(spec)
  File "<frozen importlib._bootstrap>", line 693, in _load
  File "<frozen importlib._bootstrap>", line 673, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 673, in exec_module
  File "<frozen importlib._bootstrap>", line 222, in _call_with_frames_removed
  File "/<<PKGBUILDDIR>>/.pybuild/pythonX.Y_3.5/build/swh/indexer/tests/__init__.py", line 5, in <module>
    from celery.contrib.testing.worker import _start_worker_thread
ImportError: No module named 'celery.contrib.testing'

----------------------------------------------------------------------
Ran 1 test in 0.118s

FAILED (errors=1)
E: pybuild pybuild:283: test: plugin distutils failed with: exit code=1: cd /<<PKGBUILDDIR>>/.pybuild/pythonX.Y_3.5/build; python3.5 -m nose -sv -a !db,!fs
dh_auto_test: pybuild --test --test-nose -i python{version} -p 3.5 returned exit code 13
debian/rules:7: recipe for target 'build' failed
make: *** [build] Error 25
dpkg-buildpackage: error: debian/rules build gave error exit status 2
--------------------------------------------------------------------------------
Build finished at 2018-10-22T12:42:26Z

Finished
--------

Fixing this by adding attribute ('break_stable' for example) to exclude it from the debian packaging works (for that test) [1].

But then the diff explodes elsewhere.
That's probably due to a missing version bump dependency on the swh-scheduler this time (should be v0.0.32 which introduces that missing module, that tag does not exist yet).

Failure: ImportError (No module named 'swh.scheduler.tests.celery_testing') ... ERROR
test_deposit (swh.indexer.tests.test_origin_head.OriginHead) ... ok
test_ftp (swh.indexer.tests.test_origin_head.OriginHead) ... ok
test_git (swh.indexer.tests.test_origin_head.OriginHead) ... ok
test_pypi (swh.indexer.tests.test_origin_head.OriginHead) ... ok
test_svn (swh.indexer.tests.test_origin_head.OriginHead) ... ok

======================================================================
ERROR: Failure: ImportError (No module named 'swh.scheduler.tests.celery_testing')
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/nose/failure.py", line 39, in runTest
    raise self.exc_val.with_traceback(self.tb)
  File "/usr/lib/python3/dist-packages/nose/loader.py", line 418, in loadTestsFromName
    addr.filename, addr.module)
  File "/usr/lib/python3/dist-packages/nose/importer.py", line 47, in importFromPath
    return self.importFromDir(dir_path, fqname)
  File "/usr/lib/python3/dist-packages/nose/importer.py", line 94, in importFromDir
    mod = load_module(part_fqname, fh, filename, desc)
  File "/usr/lib/python3.5/imp.py", line 234, in load_module
    return load_source(name, filename, file)
  File "/usr/lib/python3.5/imp.py", line 172, in load_source
    module = _load(spec)
  File "<frozen importlib._bootstrap>", line 693, in _load
  File "<frozen importlib._bootstrap>", line 673, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 673, in exec_module
  File "<frozen importlib._bootstrap>", line 222, in _call_with_frames_removed
  File "/<<PKGBUILDDIR>>/.pybuild/pythonX.Y_3.5/build/swh/indexer/tests/test_orchestrator.py", line 14, in <module>
    from swh.scheduler.tests.celery_testing import CeleryTestFixture
ImportError: No module named 'swh.scheduler.tests.celery_testing'

----------------------------------------------------------------------
Ran 25 tests in 0.279s

[1] Avoid breaking stable for now

diff --git a/debian/rules b/debian/rules
index 0b0f59f..93beba9 100755
--- a/debian/rules
+++ b/debian/rules
@@ -1,7 +1,7 @@
 #!/usr/bin/make -f
 
 export PYBUILD_NAME=swh.indexer
-export PYBUILD_TEST_ARGS=-sv -a !db,!fs
+export PYBUILD_TEST_ARGS=-sv -a !db,!fs,!break_stable
 
 %:
 	dh $@ --with python3 --buildsystem=pybuild
diff --git a/swh/indexer/tests/__init__.py b/swh/indexer/tests/__init__.py
index a499da6..247587e 100644
--- a/swh/indexer/tests/__init__.py
+++ b/swh/indexer/tests/__init__.py
@@ -1,21 +1,23 @@
 from os import path
 import swh.indexer
 
-from celery import shared_task
-from celery.contrib.testing.worker import _start_worker_thread
-from celery import current_app
+from celery import shared_task, current_app
+try:
+    from celery.contrib.testing.worker import _start_worker_thread
+    # Needed to pass an assertion, see
+    # https://github.com/celery/celery/pull/5111
+    def start_worker_thread():
+        return _start_worker_thread(current_app)
 
-__all__ = ['start_worker_thread']
+except ImportError:
+    pass  # stable version breakage, remove this when stable using celery > 4
 
-SQL_DIR = path.join(path.dirname(swh.indexer.__file__), 'sql')
 
+__all__ = ['start_worker_thread']
 
-def start_worker_thread():
-    return _start_worker_thread(current_app)
+SQL_DIR = path.join(path.dirname(swh.indexer.__file__), 'sql')
 
 
-# Needed to pass an assertion, see
-# https://github.com/celery/celery/pull/5111
 @shared_task(name='celery.ping')
 def ping():
     return 'pong'
diff --git a/swh/indexer/tests/test_orchestrator.py b/swh/indexer/tests/test_orchestrator.py
index 6ada2a9..a7bedae 100644
--- a/swh/indexer/tests/test_orchestrator.py
+++ b/swh/indexer/tests/test_orchestrator.py
@@ -5,14 +5,14 @@
 
 import unittest
 
+from nose.plugins.attrib import attr
+
 from swh.indexer.orchestrator import BaseOrchestratorIndexer
 from swh.indexer.indexer import BaseIndexer
 from swh.indexer.tests.test_utils import MockIndexerStorage, MockStorage
 from swh.scheduler.task import Task
 from swh.scheduler.tests.celery_testing import CeleryTestFixture
 
-from . import start_worker_thread
-
 
 class BaseTestIndexer(BaseIndexer):
     ADDITIONAL_CONFIG = {
@@ -123,8 +123,10 @@ class MockedTestOrchestrator12(TestOrchestrator12):
         self.running_tasks.extend(celery_tasks)
 
 
+@attr('break_stable')
 class OrchestratorTest(CeleryTestFixture, unittest.TestCase):
     def test_orchestrator_filter(self):
+        from . import start_worker_thread
         with start_worker_thread():
             o = TestOrchestrator12()
             o.prepare()

@attr('break_stable')

And yes, i know we are migrating away from nose.
Changing this now would also pull also some other changes so i refrained from it.
That can be solved later in another diff.

Here, i'd like we avoid breaking the debian packaging without too much changes.

But then the diff explodes elsewhere.
That's probably due to a missing version bump dependency on the swh-scheduler this time (should be v0.0.32 which introduces that missing module, that tag does not exist yet).

Yes, that was the reason. Tagged the v0.0.32 and uploaded. Then build again and all is fine (for stable).

[1] Avoid breaking stable for now

This patch is now unneeded.
Since @olasd backported the celery deps to the swh debian repository (~> that pulls an upgrade in all debian-stable workers though...).

In any case, now the build is ok:

$ bin/make-package -d stable -b swh-indexer
...
Build Architecture: amd64
Build Type: full
Build-Space: n/a
Build-Time: 4
Distribution: stretch-backports-swh
Host Architecture: amd64
Install-Time: 10
Job: /tmp/tmp.I9qZOxQLYX/swh-indexer_0.0.52.post27-1~bpo9~swh+1.dsc
Machine Architecture: amd64
Package: swh-indexer
Package-Time: 16
Source-Version: 0.0.52.post27-1~bpo9~swh+1
Space: n/a
Status: successful
This revision is now accepted and ready to land.Oct 23 2018, 11:31 AM
This revision was automatically updated to reflect the committed changes.