elsaperelli commented 1 year ago

Summary

This PR upgrades the typescript version to the latest version, 5.0.4.

New behavior

There should be no new behavior related to calculation.

Code changes

As anticipated, the only fixes involved needing to explicitly handle errors in try/catch statements.
When I upgraded the typescript version, I got a warning that the jest version we were on only supported typescript versions <5.0.0, so I decided to upgrade our jest and ts-jest versions as well. If for whatever reason we want to maintain the old jest version, then I would have to downgrade the typescript version to 4.9.5.
I also upgraded typescript-eslint dependencies.
I got a warning that ts-jest in globals was deprecated, so I changed jest.config.js to reflect the current ts-jest installation docs.

Error handling changes:

ValueSetResolver.ts - This approach is consistent to what we do in Execution.ts. The alternative is to do: catch (e: any) or catch (e: unknown).
MeasureBundleHelpers.test.ts / parseQueryInfo.test.ts - I thought this was a better approach than catch (e: any), but let me know if that doesn't make sense.

Testing guidance

npm run check Make sure there are no new warnings or errors and that all tests pass.
Make sure the changes I made to the try/catch errors make sense.

github-actions[bot] commented 1 year ago

Coverage report

St.^{:grey_question:}	Category	Percentage	Covered / Total
🟢	Statements	85.39% (-0.03% 🔻)	2023/2369
🟡	Branches	73.97% (-0.02% 🔻)	1802/2436
🟢	Functions	87.5%	357/408
🟢	Lines	85.69% (-0.04% 🔻)	1952/2278

Show files with reduced coverage 🔻

|

St.^{:grey_question:}

| File | Statements | Branches | Functions | Lines | | :----------------------------------------------------------------------------------------------------------------------------------------------------: | :------------------------------------------------------------------------------------- | :------------------------------------------------------------- | :------- | :-------- | :------------------------------------------------------------ | | 🟢 |

`...` / ValueSetResolver.ts

|

93.1% (-3.19% 🔻)

| 50% | 100% |

93.1% (-3.19% 🔻)

| | 🟡 |

`...` / ClauseResultsBuilder.ts

|

72.76% (-0.09% 🔻)

| 60.59% | 86.49% |

72.31% (-0.09% 🔻)

|

Test suite run success

379 tests passing in 29 suites.

Report generated by 🧪jest coverage report action from a05e87921cdd99ffd594ad9de6d3cd750c39c717

mgramigna commented 1 year ago

While we're here, can we finally just comment out this unused variable (don't remove it) so that lint passes without warnings? https://github.com/projecttacoma/fqm-execution/pull/219/files#file-src-calculation-clauseresultsbuilder-ts-L354

hossenlopp commented 1 year ago