ADD: Run unit tests for all pushes and PRs to develop/em using Github Actions

[fhagemann]

This is my first attempt of adding a testing environment to be used with GitHub Actions, created by building zoglauer/megalib:develop-cosi and the newest commit on cositools/nuclearizer:develop/em.
This GitHub Action is triggered with every push to develop/em or every PR onto develop/em.

Some description of what was added:

• A new file test/unit_tests.cpp which right now only checks that 1 == 1 and 2 == 2. This will then become the file to be populated with actual unit tests.
• A new target test in the Makefile to run the tests using catch.hpp
• EDIT: This is now using the unit tests that were added in ADD: First set of nuclearizer unit tests #150.
• .github/workflows/tests.yaml is the Github Actions workflow file, building megalib and nuclearizer and then executing the tests via make test. I tried to make it cache the builds of megalib (seems to work, massive speed-up starting with the second run) and nuclearizer (which does not seem to be caching much, still takes more than 30min to build). The build of megalib seem to take ~11min, and nuclearizer more than 30min.

Feedback is very welcome, as well as help with how to properly cache the nuclearizer build to avoid building that "from scratch" in every new push.

[fhagemann]

This seems to be caching megalib well (200MB cache), but not nuclearizer (13KB cache):

[fhagemann]

I removed the caching for now (can still look into it), but this is the first time I actually got things running.

I saw that the unit tests depend on the local time of the machine running the tests, so we might want to update MTime in megalib/develop-cosi to actually parse everything in UTC.

[fhagemann]

For updating MTime in megalib, see also zoglauer/megalib#121

[fhagemann]

I rebased this PR to include the changes from #159 and plan to run this action twice (once "from scratch", and then re-run to see how well the caching is doing).

[fhagemann]

Damn, so looks like the nuclearizer build now only takes ~4min instead of ~35min previously :)
Thanks @zoglauer for #159 !

Still to be determined how much we can save through caching

[fhagemann]

Ok, with caching we're down to ~3min for the whole workflow! I think is actually becoming viable now.

[fhagemann]

For some reason, it doesn't work on macos-latest out of the box, so I'm going to leave this PR to ubuntu-latest only for now and deal with macos-latest later.

[fhagemann]

Last push is just a rebase after merging #161
(and a test of what happens if megalib gets updated to see if the caching is done correctly or not).

[fhagemann]

The caching seems to have been successful: megalib:develop-cosi changed, which caused a re-build of megalib in GitHub actions rather than loading it from the cache :)

[zoglauer]

The one digit change is a clipping or rounding error, which is not good...

[fhagemann]

So, how should we deal with this? 😅

[zoglauer]

Do you know more about the system it is run on?

[fhagemann]

I can look it up. But FYI:
I also need to make this change for this to pass the tests on my Ubuntu laptop. So, it's not just GitHub actions.

[fhagemann]

The GitHub actions tests are run on

Current runner version: '2.334.0'
Runner Image Provisioner
  Hosted Compute Agent
  Version: 20260520.533
  Commit: 189110e25284a9812c124fd27b339e2fb4f2f9db
  Build Date: 2026-05-20T17:44:04Z
  Worker ID: {a1109e69-adc1-42dc-aa08-40733abc3042}
  Azure Region: eastus2
Operating System
  Ubuntu
  24.04.4
  LTS
Runner Image
  Image: ubuntu-24.04
  Version: 20260525.161.1
  Included Software: https://github.com/actions/runner-images/blob/ubuntu24/20260525.161/images/ubuntu/Ubuntu2404-Readme.md
  Image Release: https://github.com/actions/runner-images/releases/tag/ubuntu24%2F20260525.161

[zoglauer]

Let me try on these which also runs Ubuntu 24.04

[zoglauer]

OK. I can confirm - I will change MunitTest to allow a one digit +-1 difference due to numerical accuracy differences.
I will let you know when it is done.

[zoglauer]

Issue should be fixed with PR 164

[fhagemann]

Yes, PR #164 fixes this, so I can undo my last commit of adjusting the file. Thanks!

[fhagemann]

Rebasing (and removing my last commit to correct the TRA file) to include the changes from PR #164

[fhagemann]

Seems to work, let me re-run the tests to see if the caching also does its thing :)

[fhagemann]

Looks good from my side: ~19min30s when building "from scratch" and ~4min when using cached builds.
@zoglauer waiting for your final approve before merging this :)

[zoglauer]

I am not very versed in GitHub actions, so I am not the best reviewer. Maybe ask Israel too?

I did copy and paste it into Claude and ChatGPT. Here is their review:

ChatGPT:
Yes. The workflow is mostly sane, but I’d fix a few things before relying on it heavily.

Biggest issues I see

1. The triggers are broader than the name suggests

Your workflow name says develop/em, but this runs on any tag push:

push:
branches:
- develop/em
tags: '*'

Branch and tag filters are separate: branch filters apply to branch refs, tag filters apply to tag refs. So tags: '*' means every tag in the repo, not only tags reachable from develop/em. GitHub’s docs also confirm that if only branches or only tags are defined, the undefined ref type won’t run; when both are defined, both can trigger independently.

Also, release: with no types is too broad for a test workflow. I would usually write:

release:
types: [published]

GitHub supports event activity filtering via types.

If you keep both push.tags and release, publishing a release/tag can cause duplicate test runs, depending on how the tag/release is created.

2. ubuntu-latest makes this less reproducible

For a large C++/ROOT/Geant4 build, I’d avoid:

runs-on: ubuntu-latest

Use:

runs-on: ubuntu-24.04

or ubuntu-22.04 if that is what you have validated. GitHub’s hosted images and preinstalled tools are updated regularly, and -latest is a moving stable label.

3. setup-micromamba@v1 is old

The current official setup-micromamba examples use @V3, and the action documents the micromamba-shell {0} wrapper you are using.

I would change:

uses: mamba-org/setup-micromamba@v1

to:

uses: mamba-org/setup-micromamba@v3

or pin to a full commit SHA if you want supply-chain hardening. GitHub’s own docs say commit SHA pins are the safest for stability/security.

4. The pkg-config files are fragile

This part is probably the highest build-failure risk:

Libs: -L${libdir} -lhdf5 -lhdf5_cpp

For HDF5 C++, the safer order is usually:

-lhdf5_cpp -lhdf5

Also, your CCfits.pc probably should include cfitsio either directly or through Requires, e.g.:

Libs: -L${libdir} -lCCfits -lcfitsio

Right now you only check that pkg-config can find the files:

pkg-config --print-errors --exists hdf5
pkg-config --print-errors --exists CCfits

That does not prove the linker can actually link a small program. I’d add a real compile/link probe.

5. The MEGAlib cache key is probably too narrow

You cache:

${{ hashFiles('megalib/src/**', 'megalib/configure') }}

That misses possible build-affecting inputs such as headers, makefiles, config templates, scripts, data used by configure, etc. If a header changes but src/** and configure do not, you could restore a stale MEGAlib build and skip rebuilding.

I’d use a broader key, for example:

${{ hashFiles('megalib/**') }}

or a curated list that includes headers, makefiles, config files, and scripts.

6. The Symlink to MEGAlib step is not a symlink step

This step copies files:

rsync -a --ignore-existing ...

but the name says symlink. More importantly, both commands end with || true, which hides real problems. If this copy is required for tests, let it fail. If it is optional, make that explicit.

I’d change the name and remove || true:

• name: Copy Nuclearizer outputs into MEGAlib tree
  shell: micromamba-shell {0}
  run: |
  set -euo pipefail
  mkdir -p "$MEGALIB/lib" "$MEGALIB/bin"
  rm -f "${{ github.workspace }}/bin/generatelinkdef"
  rsync -a --ignore-existing "${{ github.workspace }}/lib/" "$MEGALIB/lib/"
  rsync -a --ignore-existing "${{ github.workspace }}/bin/" "$MEGALIB/bin/"
  Minimal patch I’d make
  name: Nuclearizer Tests for develop/em

on:
push:
branches:
- develop/em
tags:
- '*'
pull_request:
branches:
- develop/em
release:
types: [published]

permissions:
contents: read

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true

jobs:
build:
name: Ubuntu
runs-on: ubuntu-24.04

Then update micromamba:

• name: Setup Conda Environment
  uses: mamba-org/setup-micromamba@v3
  with:
  environment-name: cosi-env
  condarc: |
  channels:
  - conda-forge
  channel_priority: strict
  create-args: >-
  root=6.32 geant4=11.2 healpix_cxx
  pkg-config make cmake cxx-compiler ccache
  ccfits=2.6 cfitsio=4.3 hdf5=1.14
  cache-environment: true
  cache-environment-key: micromamba-${{ runner.os }}-root6.32-geant4.11.2-ccfits2.6-cfitsio4.3-hdf5.1.14

And strengthen the pkg-config step:

cat > "$CONDA_PREFIX/lib/pkgconfig/hdf5.pc" <<EOF
prefix=$CONDA_PREFIX
exec_prefix=${prefix}
libdir=${exec_prefix}/lib
includedir=${prefix}/include

Name: hdf5
Description: HDF5
Version: 1.14
Libs: -L${libdir} -lhdf5_cpp -lhdf5
Cflags: -I${includedir}
EOF

cat > "$CONDA_PREFIX/lib/pkgconfig/CCfits.pc" <<EOF
prefix=$CONDA_PREFIX
exec_prefix=${prefix}
libdir=${exec_prefix}/lib
includedir=${prefix}/include

Name: CCfits
Description: CCfits
Version: 2.6
Libs: -L${libdir} -lCCfits -lcfitsio
Cflags: -I${includedir}
EOF

export PKG_CONFIG_PATH="$CONDA_PREFIX/lib/pkgconfig:$CONDA_PREFIX/share/pkgconfig:${PKG_CONFIG_PATH:-}"

pkg-config --print-errors --exists hdf5
pkg-config --print-errors --exists CCfits
pkg-config --libs --cflags hdf5 CCfits

echo 'int main(){return 0;}' | g++ -x c++ - $(pkg-config --cflags --libs hdf5 CCfits) -o /tmp/pkgconfig-link-test
Overall verdict

I’d call this workflow close, but not fully robust yet. The main fixes are: narrow the release trigger, pin the runner, update setup-micromamba, make the pkg-config stubs link-tested, broaden the MEGAlib cache key, and stop hiding rsync failures.

Claude:
This is a well-constructed workflow for a heavy scientific build (MEGAlib + ROOT + Geant4 via conda) — the ccache setup, the conda environment caching, and the set -euo pipefail discipline are all solid. Here's what I'd change, roughly in order of impact.
Correctness / cost issues worth fixing
ccache will likely blow your cache budget and evict the MEGAlib cache. Your ccache key embeds ${{ github.sha }}, which is unique per commit, so the exact key essentially never hits — every run restores via restore-keys and then saves a brand-new cache at the post step. With CCACHE_MAXSIZE: 5G, each save can be multiple GB, and GitHub's per-repo cache limit is 10 GB total. After a couple of runs you're thrashing that limit, and least-recently-used eviction can quietly delete your expensive megalib-* cache, forcing a full MEGAlib rebuild. Two fixes that combine well:

Split into actions/cache/restore (with your restore-keys) at the start and actions/cache/save at the end, and gate the save to pushes only (if: github.event_name == 'push') so PRs don't each spawn a 5 GB entry.
Consider lowering CCACHE_MAXSIZE to ~2G; the COSI codebase almost certainly doesn't need 5 GB of cached objects.

The develop-em ccache fallback is dead. Your branch is develop/em (slash), so caches are saved under keys like ccache-Linux-develop/em-. But the second restore-key is ccache-Linux-develop-em- (dash). It never matches. The third key ccache-Linux- will still catch those caches, so it's not fatal, but the intended branch-specific fallback isn't doing anything. Change it to ccache-${{ runner.os }}-develop/em-.
release: fires on every release activity type. With no types specified, this triggers on created, edited, deleted, prereleased, published, etc. — so editing or deleting a release re-runs the whole build. You almost certainly want:
yamlrelease:
types: [published]
The MEGAlib cache gets polluted with Nuclearizer artifacts. Your "Symlink to MEGAlib" step rsyncs Nuclearizer's lib/ and bin/ into MEGALIB/lib‘and‘MEGALIB/lib and MEGALIB/lib‘and‘MEGALIB/bin. Since actions/cache saves at the post-job step (after that rsync), the run that populates the megalib cache stores Nuclearizer's binaries inside it too. It's mostly benign because Nuclearizer rebuilds anyway, but it bloats the cache and means a restored "MEGAlib" cache contains stale foreign objects. If you can, do the symlinking against a path outside the cached directories, or save the megalib cache explicitly before the rsync.
Robustness / hygiene
No concurrency control. For a build this expensive, superseded commits keep running. Add:
yamlconcurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true
No permissions block. Default-deny is best practice. At the top:
yamlpermissions:
contents: read
No timeout-minutes. If UTNRunner hangs, the job runs until the 6-hour default. A job-level timeout-minutes: 60 (tune to taste) is cheap insurance.
Unpinned MEGAlib ref. ref: develop-cosi is a moving branch, so builds aren't reproducible and a force-push upstream silently changes your CI. Your megalib cache key hashes megalib/src/, so it'll invalidate correctly, but pinning to a tag or SHA (even periodically bumped) makes failures attributable.
tags: '*' only matches tags without slashes — * doesn't cross /. Use '' if you ever cut hierarchical tags like v1/rc1. Also note that with both branches and tags under push, this fires on all tag pushes; confirm that's intended.
Minor / performance

make -j2 underutilizes the GitHub-hosted Linux runner (4 vCPUs). -j$(nproc) or -j4 should meaningfully cut build time, especially on a ccache miss.
The hand-written hdf5.pc / CCfits.pc hardcode versions (1.14, 2.6) and lib names (-lhdf5 -lhdf5_cpp). If you bump the create-args pins, these drift silently. Worth a comment tying them together, or deriving the version from the installed package.
rsync ... || true swallows real failures. If the intent is only "don't fail when source is empty," that's fine, but you lose signal if rsync breaks for another reason.
The failure-log upload globs /tmp/*.log; confirm the test harness actually writes there, otherwise the artifact is empty (v4 warns rather than errors, so it's harmless either way).
Action pinning: @v4 / @v1 major tags are fine functionally, but for supply-chain hardening many projects pin to commit SHAs. Also worth checking whether setup-micromamba has a newer major than v1 — I'd verify current versions rather than trust my memory here.

Nothing here is breaking your builds today; the ccache-budget and cache-pollution items are the ones most likely to cause confusing "why did MEGAlib rebuild from scratch again" slowdowns over time. Want me to produce a revised version of the YAML with these applied?

[fhagemann]

Thanks, I'll have a look at this in more detail later today :)

[israelmcmc]

@zoglauer @fhagemann I took a look at this as requested. I looks good, nothing weird that I can see.

Last time I had to do this (use MEGAlib for CI pipeline) I decided to go with a Docker image. I was using GitLab at that time, and that was the recommended method, but I found it very convenient and I know GitHub also supports it. It seems a little bit easier to control than the GitHub cache. But either option should be OK, just something to consider in the future,

[fhagemann]

@israelmcmc Thanks so much!
I've never used Docker in these automated tests before: would this require to push a new image everything we change something in megalib:develop-cosi, such that we can load that image here then? Or how would that work?
(For these tests we can't really use a stable version of megalib, because some fixes in nuclearizer actually require to update some things in megalib:develop-cosi as well)

[israelmcmc]

The image is the starting point of the test. It can be whatever you want. I'd create an image with a version of megalib and nuclearizer compiled, and then, during the actual test, pull the code from the pull request, recompile both of them, and run the tests. You would follow the same step as you do in your local machine. The Docker image doesn't need to change, but it would be good to do so from time to time.

[fhagemann]

@israelmcmc I might probably leave the current PR as is, now that it's working, but this is definitely something I would like to look into in the future. Do you have some example GitHub repo that has this already implemented that I could use as reference to see how they're doing it? :)

[israelmcmc]

Yes, that makes sense.

I have this one. It's in GitLab, but it should be similar for GitHub:
https://gitlab.com/burstcube/bc-tools/-/blob/develop/.gitlab-ci.yml?ref_type=heads

It loads the image burstcube/bc-tools-ci:0.1.10 and runs a series of commands, including installation and tests.

The Dockerfile that produced the image above is here: https://gitlab.com/burstcube/bc-tools/-/blob/develop/ci.Dockerfile?ref_type=heads

All of this likely outdated, but it should give you an idea.

[fhagemann]

That's perfect, I'll have a look! Thanks

[fhagemann]

In the latest commit, I addressed all of Andreas' points:

1. Only trigger this workflow on pushes and PRs onto develop/em, not on tags or releases
2. Change ubuntu-latest to ubuntu-24.04
3. Update setup-micromamba from v1 to v3
4. Update pkg-config files and prove the linker can actually link a small program.
5. Improve hash in megalib cache key a curated list that includes headers, makefiles, config files, and scripts.
6. Remove || true from the rsync commands
7. Lowered cache size to 1GB (probably could even be smaller, current caches are ~200MB)
8. Added a timeout (30 minutes) to kill workflow runs that hang
9. Update make -j2 to make -j$(nproc)

[fhagemann]

Test still pass (and are even slightly faster, probably because of the make -j$(proc)).
@zoglauer If that's fine for you, I would propose merging (or "squashing and merging" 😉 ) this as is, and I can have a look at adding macOS or transitioning from ccache to Docker images in a future follow-up PR.

[zoglauer]

Merge it!