Skip to content

Add GitHub Actions workflow to run the unit tests - #23

Merged
edefend92 merged 2 commits into
yahoo:masterfrom
edefend92:add-unit-test-github-action
Sep 1, 2026
Merged

Add GitHub Actions workflow to run the unit tests#23
edefend92 merged 2 commits into
yahoo:masterfrom
edefend92:add-unit-test-github-action

Conversation

@edefend92

Copy link
Copy Markdown
Contributor

Summary

  • This repo currently has no CI running its unit test suite on push/PR: no .github/workflows, no Travis/CircleCI/Jenkins config, and the Actions API confirms zero workflows registered (GET /repos/yahoo/NetCHASM/actions/workflowstotal_count: 0).
  • Add .github/workflows/unit-tests.yml, which builds all six CppUnit test binaries (configparsertests, coretests, externalio, logtests, storetests, worktests) and runs them via ctest on push/PR against master.
  • MDBM, Kafka, and RapidXML support are compiled out via the repo's existing SKIP-* CMake options, since those dependencies aren't packaged for stock Ubuntu. Everything else (protobuf, yaml-cpp, libevent, c-ares, cppunit, openssl/curl) installs via apt.

Test plan

  • Ran this exact configure/build/ctest sequence locally (substituting only the dependency install paths for this sandbox) — 100% tests passed, 0 tests failed out of 6 suites.

Note for maintainers

This is a new workflow file introduced via a fork PR, so per GitHub's default policy it likely needs a maintainer to click "Approve and run workflows" the first time before it executes.

There was no CI running the unit test suite for PRs against this repo
(no .github/workflows, no Travis/CircleCI/Jenkins config, and the
GitHub Actions API confirms zero workflows on yahoo/NetCHASM). Add one
that builds all six CppUnit test binaries and runs them via ctest on
push/PR against master.

MDBM, Kafka, and RapidXML support are compiled out via the existing
SKIP-* CMake options since those dependencies aren't packaged for
stock Ubuntu; everything else installs via apt. Verified locally that
this build+test sequence compiles and passes (100% tests passed, 0
failed out of 6 suites).

@brbzull0 brbzull0 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Four comments on the workflow itself. Worth noting a merge order: this should land before #22, otherwise the new tests added there run in no CI at all.

Comment thread .github/workflows/unit-tests.yml
Comment thread .github/workflows/unit-tests.yml Outdated
Comment thread .github/workflows/unit-tests.yml
Comment thread .github/workflows/unit-tests.yml

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an initial GitHub Actions CI workflow to build and run the repository’s CppUnit-based unit test suites on pushes and pull requests to master.

Changes:

  • Introduces a new Actions workflow that installs build/test dependencies on Ubuntu.
  • Configures a CMake build with selected SKIP-* options and builds the six test binaries.
  • Runs the test suites via ctest in CI.
Suppressed comments (1)

.github/workflows/unit-tests.yml:40

  • -DCOV=ON enables coverage compile/link flags, but this workflow doesn't collect coverage artifacts. If the goal is just to run unit tests, drop -DCOV=ON to keep builds closer to the default compilation flags and reduce overhead.
            -DCMAKE_BUILD_TYPE=Debug -DCOV=ON \

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .github/workflows/unit-tests.yml Outdated
Comment thread .github/workflows/unit-tests.yml
Comment thread .github/workflows/unit-tests.yml Outdated
- Add permissions: contents: read at the workflow level. The push
  trigger inherits the repo default token permissions (can be
  read-write on older repos), unlike fork pull_request runs which are
  read-only by default; this job only checks out/builds/tests.
- Pin the runner to ubuntu-24.04 instead of ubuntu-latest so a future
  image bump doesn't silently swap in a protobuf release that has
  removed pair.ByteSize().
- Drop -DCOV=ON and the lcov package: this job builds the six test
  targets directly rather than the repo's `testbuild` target, so the
  add_custom_command that generates the lcov/genhtml report never
  runs and the coverage instrumentation was dead weight.
- Rename the job from coretests to unit-tests since it builds and
  runs all six suites, not just coretests.
- Update the SKIP-* comment to also mention SKIP-IPV6.

Reviewed locally: reconfigured and rebuilt all six test targets
without -DCOV=ON and reran the full suite (100% tests passed, 0
failed out of 6).
@edefend92
edefend92 merged commit 5b80926 into yahoo:master Sep 1, 2026
@edefend92
edefend92 deleted the add-unit-test-github-action branch September 1, 2026 18:27
edefend92 added a commit to edefend92/NetCHASM that referenced this pull request Sep 1, 2026
HMStorage.h uses std::condition_variable and HMConnectionHandler.h /
HMNetCHASMAPI.h use std::shared_ptr without including the headers
that declare them. Older libstdc++ (e.g. the GCC 10 toolchain this
project has built with) happened to pull these in transitively via
other standard headers; GCC 13+ (as shipped on ubuntu-24.04, used by
the new unit-tests workflow from yahoo#23) does not, so the build fails
with "'condition_variable' in namespace 'std' does not name a type"
and "'shared_ptr' in namespace 'std' does not name a template type".

Add the missing includes directly rather than relying on transitive
inclusion.

Verified locally: built and ran the full test suite (all 6 CppUnit
suites) with both GCC 10 and GCC 14, both green.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants