online-bridge-hackathon / DDS

An api that returns DDS results for a given deal or partial deal
https://bit.ly/bridge-hackathon
Apache License 2.0
4 stars 6 forks source link

Add test simulating two simultaneous post request #50

Closed suokko closed 4 years ago

suokko commented 4 years ago

Data race from calling SetMaxThreads/SetResources multiple times got a simple fix. This is a test case testing we won't repeat same data race in future.

Signed-off-by: Pauli suokkos@gmail.com

-- The branch doesn't have fix so it can crash. This test requires two runs to have a chance to crash when I run test locally. I suspect there is some timing issues with a thread starting.

suokko commented 4 years ago

It is disconcerting that with the buggy api.py it crashes so hard that it takes the python process down with it and so does not identify the name of the failing test, but I know of no way around that.

We could add SIGSEGV handler which prints out currently running test. But I wouldn't trust python to handle the job which means it should be likely written in cpython. (signal handler and printing part) Then unittests would have to call cpython module before each tests starts running.

Does that sounds like a useful feature considring it likely needs 20-50 lines of code plus a compiled python library. If it sounds like worth a effert we should have a feature request issue about it.

suokko commented 4 years ago

Your proposals made test look much cleaner and easier to read. I should have done a personal review before posting. My excuse to skip a personal review was tiredness (late evening) and frustration after hitting many small issues how to test flask correctly (the documentation of flask could be better).

There is still the question why importing api.py requires path. My python knowledge or searching skills can't figure out answer.

tameware commented 4 years ago

I do not think it worth the effort to add a SIGSEGV handler. I was just kvetching…