785414 - Fix ES test case bit rot

Status: RESOLVED FIXED

(Marketplace Graveyard :: Code Quality, defect)

Description

[Kumar McMillan [:kumar]]

We can run ES tests in CI (bug 784051), woo! Next: spray down all ES tests with anti-bit-rot, let sit for an hour, rinse, and repeat until all bit rot is gone.

Comment 1

[Christopher Van Wiemeersch [:cvan]]

How is this different from bug 784047?

Comment 2

[Kumar McMillan [:kumar]]

The bit rot tests were broken before the gaia-mobile merge ;)

Comment 3

[Christopher Van Wiemeersch [:cvan]]

I see some broken AMO ones too (from just `addons` not `search`):

======================================================================
ERROR: test_job (zamboni3.apps.addons.tests.test_cron.TestReindex)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/chris/Sites/projects/zamboni3/../zamboni3/apps/addons/tests/test_cron.py", line 238, in test_job
    self.refresh()
  File "/Users/chris/Sites/projects/zamboni3/apps/amo/tests/__init__.py", line 593, in refresh
    cls.es.refresh(settings.ES_INDEXES[index], timesleep=timesleep)
  File "/Users/chris/Sites/projects/zamboni3/vendor/lib/python/pyes/es.py", line 462, in refresh
    result = self._send_request('POST', path)
  File "/Users/chris/Sites/projects/zamboni3/vendor/lib/python/pyes/es.py", line 223, in _send_request
    raise_if_error(response.status, decoded)
  File "/Users/chris/Sites/projects/zamboni3/vendor/lib/python/pyes/convert_errors.py", line 68, in raise_if_error
    raise excClass(msg, status, result)
IndexMissingException: [test_amo] missing

======================================================================
ERROR: test_create (zamboni3.apps.addons.tests.test_models.TestSearchSignals)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/chris/Sites/projects/zamboni3/../zamboni3/apps/addons/tests/test_models.py", line 1806, in test_create
    self.refresh()
  File "/Users/chris/Sites/projects/zamboni3/apps/amo/tests/__init__.py", line 593, in refresh
    cls.es.refresh(settings.ES_INDEXES[index], timesleep=timesleep)
  File "/Users/chris/Sites/projects/zamboni3/vendor/lib/python/pyes/es.py", line 462, in refresh
    result = self._send_request('POST', path)
  File "/Users/chris/Sites/projects/zamboni3/vendor/lib/python/pyes/es.py", line 223, in _send_request
    raise_if_error(response.status, decoded)
  File "/Users/chris/Sites/projects/zamboni3/vendor/lib/python/pyes/convert_errors.py", line 68, in raise_if_error
    raise excClass(msg, status, result)
IndexMissingException: [test_amo] missing

======================================================================
ERROR: test_delete (zamboni3.apps.addons.tests.test_models.TestSearchSignals)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/chris/Sites/projects/zamboni3/../zamboni3/apps/addons/tests/test_models.py", line 1825, in test_delete
    self.refresh()
  File "/Users/chris/Sites/projects/zamboni3/apps/amo/tests/__init__.py", line 593, in refresh
    cls.es.refresh(settings.ES_INDEXES[index], timesleep=timesleep)
  File "/Users/chris/Sites/projects/zamboni3/vendor/lib/python/pyes/es.py", line 462, in refresh
    result = self._send_request('POST', path)
  File "/Users/chris/Sites/projects/zamboni3/vendor/lib/python/pyes/es.py", line 223, in _send_request
    raise_if_error(response.status, decoded)
  File "/Users/chris/Sites/projects/zamboni3/vendor/lib/python/pyes/convert_errors.py", line 68, in raise_if_error
    raise excClass(msg, status, result)
IndexMissingException: [test_amo] missing

======================================================================
ERROR: test_update (zamboni3.apps.addons.tests.test_models.TestSearchSignals)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/chris/Sites/projects/zamboni3/../zamboni3/apps/addons/tests/test_models.py", line 1812, in test_update
    self.refresh()
  File "/Users/chris/Sites/projects/zamboni3/apps/amo/tests/__init__.py", line 593, in refresh
    cls.es.refresh(settings.ES_INDEXES[index], timesleep=timesleep)
  File "/Users/chris/Sites/projects/zamboni3/vendor/lib/python/pyes/es.py", line 462, in refresh
    result = self._send_request('POST', path)
  File "/Users/chris/Sites/projects/zamboni3/vendor/lib/python/pyes/es.py", line 223, in _send_request
    raise_if_error(response.status, decoded)
  File "/Users/chris/Sites/projects/zamboni3/vendor/lib/python/pyes/convert_errors.py", line 68, in raise_if_error
    raise excClass(msg, status, result)
IndexMissingException: [test_amo] missing

======================================================================
FAIL: test_no_addons (zamboni3.apps.addons.tests.test_models.TestSearchSignals)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/chris/Sites/projects/zamboni3/../zamboni3/apps/addons/tests/test_models.py", line 1802, in test_no_addons
    eq_(Addon.search().count(), 0)
AssertionError: <MagicMock name='get_es().search().__getitem__().__getitem__()' id='4427294096'> != 0

Comment 4

[Kumar McMillan [:kumar]]

if you run those test classes directly then everything passes; there is something wrong with setup/teardown or something is leaky.

Comment 5

[Kumar McMillan [:kumar]]

We're still seeing the leaky setup/teardown problem. If you run, e.g., browse/tests/test_views.py:TestCategoryLanding.test_bad_cat it passes on its own but if you run the whole suite it fails with the missing index error.

Comment 6

[Wil Clouser [:clouserw]]

I suspect Wraithan knows more about ES than he cares to - maybe he can help here.  Kumar - is this on your radar at all or should we reassign?  The payments stuff is still P1.

Comment 7

[Kumar McMillan [:kumar]]

Not on my radar. I'm happy to advise. Comment #4 is where anyone else can pick up and run.

Comment 8

[Tarek Ziadé (:tarek)]

Unfortunately running test_models here results in a lot of errors...


$ python manage.py test apps.addons.tests.test_models
[...]
Ran 236 tests in 158.773s
FAILED (errors=23, failures=3)

digging

Comment 9

[Tarek Ziadé (:tarek)]

I had many other issues trying to run the ES tests, related to "browse." being replaced by mkt.browse and then the imports starting to browse.* in the apps looking in mkt.browse instead of looking in apps.

I have spent too much time on this and I am not even sure I have the right environment to run all tests correctly. I will try to seek other tasks to be more productive

Comment 10

[Tarek Ziadé (:tarek)]

Running the search tests alone I get yet another set of errors: http://tarek.pastebin.mozilla.org/1869866

Comment 11

[Tarek Ziadé (:tarek)]

FWIW I had a lot of "org.apache.lucene.store.LockObtainFailedException" in ES and those got fixed by adding 

index.store.fs.lock: none

in the configuration file.

That puts me down to FAILED (SKIP=14, errors=1, failures=4) in mkt.search

Comment 12

[Tarek Ziadé (:tarek)]

Just adding more notes here to share

this one breaks in isolation / mkt.search.tests.test_views:TestWebappSearch.test_unknown_cat

the search page returns 200 instead of 302 and happens to be a "Additional authorization required" screen.

Comment 13

[Tarek Ziadé (:tarek)]

Well... the code that supposes to do the redirect was commented out

=> https://github.com/mozilla/zamboni/blob/master/mkt/search/views.py#L182

what should I do ? nuke the test or reactivate this code ?

Comment 14

[Tarek Ziadé (:tarek)]

So it looks like it was old-e code https://github.com/mozilla/zamboni/commit/1b3214b4a1d4df920fb9c38aefefdaeb690f21a8

I am +1 for nuking the test and the commented code. But I need a supporter :)

Comment 15

[Tarek Ziadé (:tarek)]

I found the leak \o/

the problem is that the mocking is not properly applied to all flavors of the get_es() method



(Pdb) from elasticutils.contrib.django import get_es 
(Pdb) get_es()
<MagicMock name='get_es()' id='102368912'>
(Pdb) from elasticutils import get_es
(Pdb) get_es()
<pyes.es.ES object at 0x84ec090>
(Pdb) import elasticutils.contrib.django
(Pdb) elasticutils.contrib.django
<module 'elasticutils.contrib.django' from '/home/marketplace/.virtualenvs/marketplace2/lib/python2.6/site-packages/elasticutils/contrib/django/__init__.pyc'>
(Pdb) import elasticutils
(Pdb) elasticutils
<module 'elasticutils' from '/home/marketplace/.virtualenvs/marketplace2/lib/python2.6/site-packages/elasticutils/__init__.pyc'>

(Pdb) sys.modules['elasticutils'].get_es()
<pyes.es.ES object at 0x84ecc90>
(Pdb) sys.modules['elasticutils.contrib.django'].get_es()
<MagicMock name='get_es()' id='102368912'>


Digging to understand why

Comment 16

[Tarek Ziadé (:tarek)]

If I replace all:

  from elasticutils.contrib.django as elasticutils

by 

  from elasticutils import get_es


I get the proper behavior. I guess the problem is that we 
cache a Mocked version of the connector at

https://github.com/mozilla/elasticutils/blob/master/elasticutils/contrib/django/__init__.py#L78

and we serve it back after the patch has been set of...

I am going to add a bit of logic to wipe out _local.es from with mock_es

Comment 17

[Tarek Ziadé (:tarek)]

r? https://github.com/mozilla/zamboni/pull/466

Comment 18

[Tarek Ziadé (:tarek)]

pushed at https://github.com/mozilla/zamboni/commit/66ab7756fabc7cae87cf216b62c4a798b1d96301

I am keeping this one open since it has an extra failure to take care of

Comment 19

[Tarek Ziadé (:tarek)]

the other one: https://github.com/mozilla/zamboni/commit/082346fded018859a4bcceb71f8e3644bd7d001f

Comment 20

[Kumar McMillan [:kumar]]

I still see some bitrotted tests on the ES builder https://ci.mozilla.org/job/marketplace-es/ Are those not failing for you locally?

Comment 21

[Kumar McMillan [:kumar]]

That builder is not green yet but the failures have their own bugs so I'll close this one.