fedora-infra / bodhi

Bodhi is a web-system that facilitates the process of publishing updates for a Fedora-based software distribution.
https://bodhi.fedoraproject.org
GNU General Public License v2.0
151 stars 189 forks source link

Ensure non-mocked message publishing during tests fails #5634

Closed AdamWill closed 2 months ago

AdamWill commented 3 months ago

In working on #5630 I found that what happens when you don't properly mock out message publish attempts in tests isn't great. In CI, the test will hang for several minutes, then eventually proceed. Obviously we don't want to slow the tests down, and there is a 30 minute time limit on the test, so if you do this more than twice or so, the tests will time out. It's not very clear why the tests are timing out and failing - you have to guess that it's message publishing causing the problem.

In bcd, the test doesn't hang, it just silently proceeds (I guess the message gets published to the rabbitmq instance in the bcd environment, I haven't checked). So you will not notice that you got it wrong until you submit a PR.

Neither of those behaviors is great, so instead, let's mock out the 'real' message publishing function in fedora_messaging in a fixture that's automatically used by every test. We replace it with a mock that raises an exception with a hopefully- useful message. We have to mock _twisted_publish, not publish, because mock_sends also mocks _twisted_publish; if we mocked publish, we would break mock_sends. This way mock_sends gets to override us and work OK.

Unfortunately this usually causes a database rollback error so you have to scroll back through some confusing errors to find the 'real' problem, but it's still an improvement, I think.

AdamWill commented 3 months ago

To try this out, you can just remove a usage of mock_sends in some test so the test will really try and publish the message, and run that test. You should see the exception is raised.