Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Subtle bug in Code Coverage CI workflow #1418

Closed
2 tasks done
thnkslprpt opened this issue Sep 23, 2023 · 0 comments · Fixed by #1419
Closed
2 tasks done

Subtle bug in Code Coverage CI workflow #1418

thnkslprpt opened this issue Sep 23, 2023 · 0 comments · Fixed by #1419

Comments

@thnkslprpt
Copy link
Contributor

Checklist

  • I reviewed the Contributing Guide.
  • I performed a cursory search to see if the bug report is relevant, not redundant, nor in conflict with other tickets.

Describe the bug
It seems that the intent of the standalone-build.yml workflow is to only run the 'Enforce coverage minimum' steps in the debug-ubuntu-20.04 build (there is a separate check for the release builds - osal_apicheck). However, I think they are currently running in all builds (incl. the 2 release builds in the matrix).

Therefore, they are referencing non-existent variables, because in the release and debug-ubuntu-22.04 builds the ncov_lines/functions/branches variables are never created.

The reason the checks normally 'pass' silently in those 3 builds is because they are comparing a non-existent variable (which they seem to treat as zero?) as greater than the env.allowed_ncov_functions/lines/branches variables (which will always evaluate to false as they obviously cannot be negative).

If you change any/all of the allowed_ncov_lines/branches/functions variables to -1, suddenly all 4 builds will fail (but the non-existent ncov_lines/functions/branches variables will not be able to be printed in 3/4 of the builds. This shows that these steps are running in all builds, whether intentionally or not:

Screenshot 2023-09-23 15 08 28

They do print correctly (see inside the brackets at the end of the error strings) with the debug-20.04 build though. As mentioned, this is the only build in which the coverage data is actually created:

- name: Install Coverage Analysis Tools
if: ${{ matrix.build-type == 'Debug' && matrix.base-os == 'ubuntu-20.04' }}
run: sudo apt-get install -y lcov xsltproc && echo "run_lcov=TRUE" >> $GITHUB_ENV

- name: Check Coverage
id: stats
if: ${{ env.run_lcov == 'TRUE' }}
uses: ./source/.github/actions/check-coverage
with:
binary-dir: build

Screenshot 2023-09-23 18 16 52

Code snips
Here are the coverage checks:

- name: Enforce coverage function minimum
if: ${{ always() && steps.stats.outputs.ncov_functions > env.allowed_ncov_functions }}
run: |
echo "::error::Too many uncovered functions (${{ steps.stats.outputs.ncov_functions }})"
/bin/false
- name: Enforce coverage line minimum
if: ${{ always() && steps.stats.outputs.ncov_lines > env.allowed_ncov_lines }}
run: |
echo "::error::Too many uncovered lines (${{ steps.stats.outputs.ncov_lines }})"
/bin/false
- name: Enforce coverage branch minimum
if: ${{ always() && steps.stats.outputs.ncov_branches > env.allowed_ncov_branches }}
run: |
echo "::error::Too many uncovered branches (${{ steps.stats.outputs.ncov_branches }})"
/bin/false

Expected behavior
The coverage checks in the code snippet above should be set to run only on the debug builds (no reason not to add them in for ubuntu-22.04 as well, from what I can see), and not run on the release builds.

Reporter Info
Avi Weiss   @thnkslprpt

thnkslprpt added a commit to thnkslprpt/osal that referenced this issue Sep 23, 2023
thnkslprpt added a commit to thnkslprpt/osal that referenced this issue Mar 23, 2024
thnkslprpt added a commit to thnkslprpt/osal that referenced this issue Aug 8, 2024
dzbaker added a commit that referenced this issue Aug 16, 2024
…orkflow-bug-fix-and-enforce-updating

Fix #1417, Fix #1418, Fix workflow bug and enforce updating of coverage minimums
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant