Skip to content
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

TestPailgunServer.test_process_request_error is flaky #6877

Closed
stuhood opened this issue Dec 6, 2018 · 1 comment
Closed

TestPailgunServer.test_process_request_error is flaky #6877

stuhood opened this issue Dec 6, 2018 · 1 comment
Assignees

Comments

@stuhood
Copy link
Sponsor Member

stuhood commented Dec 6, 2018

__ TestPailgunServer.test_process_request_error __
 
 self = <pants_test.pantsd.test_pailgun_server.TestPailgunServer testMethod=test_process_request_error>
 mock_shutdown_request = <function shutdown_request at 0x7fe39f1f0140>
 
     @mock.patch.object(PailgunServer, 'shutdown_request', **PATCH_OPTS)
     def test_process_request_error(self, mock_shutdown_request):
       mock_request = mock.Mock()
       self.mock_handler_inst.handle_request.side_effect = AttributeError('oops')
       self.server.process_request(mock_request, ('1.2.3.4', 31338))
 >     self.assertIs(self.mock_handler_inst.handle_request.called, True)
 E     AssertionError: False is not True
 
 pants_test/pantsd/test_pailgun_server.py:64: AssertionError
@jsirois
Copy link
Member

jsirois commented Dec 10, 2018

I hit this. A quick look says this test - and others in the same class - are doomed to failure. They exercise self.server.process_request which is a method that spawns a thread with no joining or latching or ... so the thing depends on the spawned thread beating the assert.

@jsirois jsirois self-assigned this Dec 10, 2018
jsirois added a commit to jsirois/pants that referenced this issue Dec 10, 2018
The `process_request` method is inherently racey to test; it's library
code that spawns a thread and provides no handle to it. Switch the
tests to exercise the underlying `process_request_thread` since that is
what we implement anyhow and which has no threading.

Fixes pantsbuild#6877
jsirois added a commit that referenced this issue Dec 11, 2018
The `process_request` method is inherently racey to test; it's library
code that spawns a thread and provides no handle to it. Switch the
tests to exercise the underlying `process_request_thread` since that is
what we implement anyhow and which has no threading.

Fixes #6877
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants