Trying out GHA for coverage and debug builds

[Myoldmopar]

Pull request overview

I'm trying out GHA for coverage and debug builds now that @jmarrec made some huge improvements to our CI system and runtime. If it goes real smooth, great. If not, this can be tossed for now.

• Fixes #ISSUENUMBERHERE

Description of the purpose of this PR

Pull Request Author

• Title of PR should be user-synopsis style (clearly understandable in a standalone changelog context)
• Label the PR with at least one of: Defect, Refactoring, NewFeature, Performance, and/or DoNoPublish
• Pull requests that impact EnergyPlus code must also include unit tests to cover enhancement or defect repair
• Author should provide a "walkthrough" of relevant code changes using a GitHub code review comment process
• If any diffs are expected, author must demonstrate they are justified using plots and descriptions
• If changes fix a defect, the fix should be demonstrated in plots and descriptions
• If any defect files are updated to a more recent version, upload new versions here or on DevSupport
• If IDD requires transition, transition source, rules, ExpandObjects, and IDFs must be updated, and add IDDChange label
• If structural output changes, add to output rules file and add OutputChange label
• If adding/removing any LaTeX docs or figures, update that document's CMakeLists file dependencies

Reviewer

• Perform a Code Review on GitHub
• If branch is behind develop, merge develop and build locally to check for side effects of the merge
• If defect, verify by running develop branch and reproducing defect, then running PR and reproducing fix
• If feature, test running new feature, try creative ways to break it
• CI status: all green or justified
• Check that performance is not impacted (CI Linux results include performance check)
• Run Unit Test(s) locally
• Check any new function arguments for performance impacts
• Verify IDF naming conventions and styles, memos and notes and defaults
• If new idf included, locally check the err file and other outputs

[jmarrec]

AFAIK, pip install gcovr pulls in 8.3, while apt on ubuntu 24.04 pulls in 7.0

[jmarrec]

I'm confused by the use the lcov and genhtml here. What's the point of installing gcovr then?

I have an older coverage test repo:
https://github.com/jmarrec/TestCpp-GHA-Coverage/blob/a7681924a92306a05a9d8486f8f1bdcd079b6239/.github/workflows/build.yml#L86-L99

gcovr -j $(nproc) --delete --root ../ --exclude '.*GTest.cpp' --print-summary --xml-pretty --xml coverage.xml .

So I guess you'd do something like this?

gcovr -j $(nproc) --delete --root ../  --exclude '.*unit.cc' --print-summary --html --html-details -o coverage.html

Maybe could do somthing like this too to make a table on the workflow summary page?

- name: Coverage summary
  shell: bash
  working-directory: ./build
  run: |
    gcovr --root ../ --txt-summary -o coverage_summary.txt
    echo "## Code Coverage" >> $GITHUB_STEP_SUMMARY
    echo '```' >> $GITHUB_STEP_SUMMARY
    cat coverage_summary.txt >> $GITHUB_STEP_SUMMARY
    echo '```' >> $GITHUB_STEP_SUMMARY

[jmarrec]

gcovr gained a markdown option, unreleased yet.

pip install git+https://github.com/gcovr/gcovr.git

gcovr --root ../ --markdown-summary >> $GITHUB_STEP_SUMMARY

---

GCC Code Coverage Report

📂 Overall coverage

Metric	Coverage
Lines	🟢 35/35 (100.0%)
Functions	🟢 13/13 (100.0%)
Branches	🔴 31/120 (25.8%)

[jmarrec]

Here is an updated workflow: https://github.com/jmarrec/TestCpp-GHA-Coverage/blob/366ac48d843a8ef997c258a20418eba3bae37b00/.github/workflows/build.yml#L98-L111

https://github.com/jmarrec/TestCpp-GHA-Coverage/actions/runs/17547795332#summary-49833174084

[jmarrec]

Here is an updated workflow: https://github.com/jmarrec/TestCpp-GHA-Coverage/blob/366ac48d843a8ef997c258a20418eba3bae37b00/.github/workflows/build.yml#L98-L111

https://github.com/jmarrec/TestCpp-GHA-Coverage/actions/runs/17547795332#summary-49833174084

[jmarrec]

Eventually I think you'd be doing something like

    mkdir html
    gcovr -j $(nproc) \
      --gcov-ignore-errors=source_not_found \
      --gcov-ignore-errors=no_working_dir_found \
      --gcov-ignore-parse-errors=suspicious_hits.warn_once_per_file \
      --merge-mode-functions=merge-use-line-min \
      --print-summary \
      --html=html/index.html --html-details \
      --markdown=summary.md \
      --root ../src/EnergyPlus \
      --exclude '.*unit.cc' \
      --exclude '.*test/.*' \
      --exclude '/usr/.*' \
      --gcov-object-directory=.
    cat summary.md >> $GITHUB_STEP_SUMMARY

[Myoldmopar]

Alright, this is looking OK. @jmarrec may be able to do some fun tweaks to get gcovr working better, but I was finally able to get it to "work". Some quick notes:

• Yes, the lcov/gcovr/genhtml operations are a little hacky. They appear to be working properly, but there is certainly a better way, and I just couldn't get any of the other combinations of command line arguments to work happily. I acknowledge someone could come along and clean this up given some time.
• I did not end up adding anything to the GITHUB_STEP_SUMMARY, or posting anything to the PR. It would be great to do that, but I'm not sure how much anyone paid attention to the exact line coverage before anyway.
• It would be great to address the massive number of fatal error lines that are not covered by unit tests. I personally think the best plan would be to figure out which code paths are impossible (based on the current schema text), and ignore them. Obviously if it's an actually impossible code path, then just remove the code.
• I skipped the top 10 longest running integration tests. It cut the run time down substantially. As of now, it's a 2 hour run, which is probably acceptable for PR commits.
• But on that note, if it starts eating up CI time, you might have to just defer this to only commits to the develop branch. It would stink to find things right after they merge, but it would be a decent compromise.
• One thing that should definitely be done is re-enabling ccache on this. I did not have the time to figure out the right cache key for this workflow, and I didn't want to break anything already working with caching, so I left it. But that could take 10-15 minutes off a typical run just by adding that in.
• This enables debug and coverage runs, which is 2/3 things that Decent was doing for us. It does not do performance test suite. Those should be run on a release build anyway. And since the performance suite process was developed like a decade ago, it's probably better to take a fresh look at the process.

OK, that's about enough. I will make a commit to resolve the merge conflict, and clean things up a little. But I would like to call this done so that Decent CI can be done for us as well.

[Myoldmopar]

This is all happy, however, I did see some CI runs were queued waiting on runners. It may have been coincidence, but it's a strong reminder of why I have deferred this in the past. For the sake of all the other github.com/NREL/ projects running Actions, I think it might be a good idea to push this to just develop commits. As I mentioned - it stinks that it means you may not find an array bounds issue, or an infinity, until the PR is merged. But if this ends up eating all the CI runners available on a busy day, the problems are much worse.

I'll note that if the ccache stuff is enabled, it may take this down from 123 minutes to like 105. That is great, but it's still a lot. There could also be some intelligence to skip this run if only documentation files are changed or something, but that would be such a small portion of the PRs, it is probably not worth it.

The best solution is to stand up a large runner, or a self-hosted runner, that can run these long-duration commits for E+ without dragging down the overall pool of NREL GHA runners.

[mjwitte]

How about a commit keyword that could run these upon request on the PR branch?

[shorowit]

The OpenStudio project uses a "Pull Request - Ready for CI" label on the PR to trigger longer CI runs, that's another option. See https://github.com/NREL/OpenStudio/pulls

[nrel-bot-2]

@Myoldmopar it has been 9 days since this pull request was last updated.

[nrel-bot-2c]

@Myoldmopar it has been 7 days since this pull request was last updated.

[nrel-bot-2]

@Myoldmopar it has been 8 days since this pull request was last updated.