search

pez-globo / pufferfish-software

All software for the Pufferfish ventilator.
Apache License 2.0
0 stars 1 forks source link

Implement unit test cases for firmware protocols #269

Closed rohanpurohit closed 3 years ago

rohanpurohit commented 3 years ago

This PR: Includes unit test cases for:

rohanpurohit commented 3 years ago

rebased on top of develop, fixed build errors after https://github.com/pez-globo/pufferfish-software/commit/917ac6302ce011d83b881628b6083f6b4d406e88

ethanjli commented 3 years ago

I'm seeing that CMakeLists has been updated to add code coverage tooling. This would explain why the Github Actions tests which involve doing CMake builds (scan-build and catch2) are failing on the "Run CMake" step - for example:

CMake Error at cmake/CodeCoverage.cmake:204 (message):
  lcov not found! Aborting...

As a fix, can you try editing https://github.com/pez-globo/pufferfish-software/blob/develop/.github/workflows/firmware.yml to add lcov to the sudo apt-get install commands in lines 94 (for the scan-build job), 141 (for the catch2-clang-tidy job), and 165 (for the catch2 job) and doing a commit+push to see if the Github Actions checks will pass afterwards?

rohanpurohit commented 3 years ago

I'm seeing that CMakeLists has been updated to add code coverage tooling. This would explain why the Github Actions tests which involve doing CMake builds (scan-build and catch2) are failing on the "Run CMake" step - for example:

CMake Error at cmake/CodeCoverage.cmake:204 (message):
  lcov not found! Aborting...

As a fix, can you try editing https://github.com/pez-globo/pufferfish-software/blob/develop/.github/workflows/firmware.yml to add lcov to the sudo apt-get install commands in lines 94 (for the scan-build job), 141 (for the catch2-clang-tidy job), and 165 (for the catch2 job) and doing a commit+push to see if the Github Actions checks will pass afterwards?

Yep lcov is required, will try that ! Thanks

rohanpurohit commented 3 years ago

yes the suggested changes worked @ethanjli , will fix format and catch2-clang-tidy as well

rohanpurohit commented 3 years ago

// resolved.

Added changes for issue #279 https://github.com/pez-globo/pufferfish-software/pull/269/commits/54c0897580377c93abd01bda3d8d5c0acb65f720

do we still need to use this function https://github.com/pez-globo/pufferfish-software/blob/develop/firmware/ventilator-controller-stm32/Core/Inc/Pufferfish/Application/States.tpp#L23 for backend.tpp ?

if yes then in terms of test scenarios, we wont be able to test different message types for StateSynchroniser::output, as we would have to then implement that in states::input maybe https://github.com/pez-globo/pufferfish-software/pull/269/commits/3891af43ed8e90940a3a213c37dd0f3d534ccdea#diff-758fdfe036be60a3797af2c480e7dee6ca6a9263f42fc8186670345f53826e73R69 return invalid_type in the switch block or rewrite the input method to incorprate it.

rohanpurohit commented 3 years ago

Closing this PR (these tests are merged in parts separately in other PR's) : Tests for CRCElements: #281 handling data input for states: #289 handling of delimiter in chunks: #290 Tests for Datagrams: #285 Tests for chunks: #297 Tests for Messages: #298 Tests for states: #306