734217 - TestSeriesSecurity tests are annoying me

Status: RESOLVED FIXED

(addons.mozilla.org Graveyard :: Code Quality, defect)

Description

[Andy McKay]

The quite regularly fail. Then pass. Then fail. They flip flop more than fish gasping for air. I'm not quite sure what the issue is. But we need to stop it happening.

Comment 1

[Rob Hudson [:robhudson]]

I tried to figure these out but can't reproduce it locally.

I've used jbalogh's hangserver.py file [1] on the ES port and still couldn't get these to fail. But I can keep trying.  I think what happened was it would check the health, not get a response, then turn these tests off.  We need the health check to pass but then hang on the rest.

[1] https://gist.github.com/527442

I've also searched for ES timeout issues to see if there's a common problem. It seemed common to increase the timeout up to 20 to 30 seconds for data sets that may make ES churn.

Comment 2

[Andy McKay]

If I understand the tests, they actually only test that there are 200 or 403 errors for all those urls. And it looks like there are an awful lot of those URLs. The contents of ES are not relevant. If thats the case... why bother talking to ES at all? Just mock ES out of the equation, its not relevant to the test which is testing the view code.

Comment 3

[Rob Hudson [:robhudson]]

This is very close:
https://github.com/robhudson/zamboni/commit/ba24ca5 

The last bit is 2 things...

1. https://github.com/mozilla/zamboni/blob/master/apps/editors/tests/test_views.py#L1894

This test is flaky and I caught it bleeding over to other tests somehow. When I remove the patch line things work as expected.

2. https://github.com/mozilla/zamboni/blob/master/apps/editors/tests/test_views.py#L249

This one now fails because of the Mock upgrade. I've scanned the code here quickly but I think I'll need to dive deeper to understand what this test is testing before knowing how to fix it.

Comment 4

[Rob Hudson [:robhudson]]

We'll see if this works. This'll mock ES for all tests that aren't subclassing ESTestCase or that don't explicitly say to `mock_es`.

https://github.com/mozilla/zamboni/commit/db004432

Mock v0.8 had a few backwards incompatibilities in that it now returns a MagicMock by default.