awslabs / amazon-kinesis-video-streams-pic

Apache License 2.0
49 stars 51 forks source link

address compiler warnings, cleanup ci #232

Closed hassanctech closed 10 months ago

hassanctech commented 10 months ago

Issue #, if available: n/a

Description of changes:

  1. Remove unnecessary double compiles in CI
  2. Move test ci run into a its own step
  3. Address many compiler warnings (zero warnings on mac and linux builds now with gcc and clang)
  4. Modify build of test project to no longer re-build all of PIC again in order to set custom compile options for the test, since using add_subdirectory propagated the flag from PIC itself down to the test project so it will always match.
  5. Refactor test project to be in it's own high level tst directory. In addition we build and run tests for PIC with two configurations with and without the flags: ALIGNED_MEMORY_MODEL and FIXUP_ANNEX_B_TRAILING_NALU_ZERO. Previously the test project enabled both flags and we did not test the path with both flags disabled (i.e. the default configuration of this library). In doing so many tests needed to be if/def'd behind these flags because they other wise crash or report the unaligned access by the usbsan run. In the run with the aforementioned flags set, exactly the same suite of tests as before run so there has been no reduction in test coverage, rather what we have now is a superset of what we had previously.
  6. Fix threadpool issue where in threadpoolInternalInactiveThreadCount we may subtract two unsigned numbers (A - B) where A < B and return the result. Now we return 0 in such cases.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

codecov-commenter commented 10 months ago

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (2dc0a16) 75.13% compared to head (cf1a2b5) 78.30%.

:exclamation: Current head cf1a2b5 differs from pull request most recent head c67aef7. Consider uploading reports for the commit c67aef7 to get more accurate results

Files Patch % Lines
src/heap/src/HybridFileHeap.c 88.88% 1 Missing :warning:
src/utils/src/Threadpool.c 50.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #232 +/- ## =========================================== + Coverage 75.13% 78.30% +3.17% =========================================== Files 52 52 Lines 10230 10032 -198 =========================================== + Hits 7686 7856 +170 + Misses 2544 2176 -368 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.