Fallback logic in /Terms/GetById hides unit test failures.

[blairlearn]

Issue description

The ESTermsQueryService.GetById method throws APIErrorException when the term is not found.

In TermsController.GetById, the fallback logic naively catches Exception as a "normal exception" and swallows it before trying to lookup the next combination.

In TermsControllerTests, the test cases GetById_WithFallback_GeneticsHP and GetById_WithFallback_TermsPatient set up mock instances of ITermsQueryService. The mocks include assertions of the order in which the various combinations are tried.

The assertions are incorrect with "off by one" errors (assuming the first callback to be number 1 instead of number 0), however the failures are swallowed by controller's logic and the failures are not reported. Because all exceptions are swallowed, no failure in checking the call order would be caught.

An "easy" fix would be to change the controller to explicitly catch APIErrorException, however this continues the use of "normal exceptions."

A better fix would be to refactor ESTermsQueryService.GetById to return null when no term is found and modify the controller accordingly.

ESTIMATE 2

Steps to reproduce the issue

1. Open the API project in a debugger.
2. In TermsController.GetById, set a breakpoint in the catch block of the fallback handling section of the code. (Lines 112-116 in the present code.)
3. In TermsControllerTest.cs, debug the GetById_WithFallback_GeneticsHP test and allow it to run.

What's the expected result?

• In the current design, the catch block should be catching APIErrorException.

What's the actual result?

• Xunit.Sdk.EqualException is caught and swallowed without being reported.

Additional details / screenshot

Related Tickets

• Issue #38

[blairl-nih]

This same issue also occurs on GetByName

[blairlearn]

Fixed in 6f47aa51bc26f886235a81ddd26ab45e5a6b2e6c