Migrate CI from CircleCI to GitHub Actions and modernize test infrastructure#1
Conversation
Co-authored-by: chenpeizhi <8114085+chenpeizhi@users.noreply.github.com>
Co-authored-by: chenpeizhi <8114085+chenpeizhi@users.noreply.github.com>
Co-authored-by: chenpeizhi <8114085+chenpeizhi@users.noreply.github.com>
Co-authored-by: chenpeizhi <8114085+chenpeizhi@users.noreply.github.com>
|
@copilot Fix the CI build and streamline the build and CI:
|
Co-authored-by: chenpeizhi <8114085+chenpeizhi@users.noreply.github.com>
Fixed in commit 614ef50: ✅ Updated Catch2 from v2.0.1 to v3.4.0 using CMake FetchContent The build now uses Catch2 v3.4.0 which is compatible with modern glibc and includes a stream insertion operator for fbitset types to ensure compatibility with Catch2 v3's type detection. |
…ructure Co-authored-by: chenpeizhi <8114085+chenpeizhi@users.noreply.github.com>
… infrastructure" This reverts commit fd84ee4.
There was a problem hiding this comment.
Pull Request Overview
This PR modernizes the project's testing and CI infrastructure by migrating from Catch2 v2 to v3, transitioning from CircleCI to GitHub Actions, and updating repository references to the new organization.
Key Changes:
- Upgraded test framework from Catch2 v2 (single-header) to v3.11.0 using CMake FetchContent
- Replaced CircleCI with GitHub Actions CI/CD pipeline supporting Debug and Release builds
- Updated repository URLs from tschijnmo to DrudgeCAS organization
Reviewed Changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/CMakeLists.txt | Migrated from downloading single-header Catch2 v2 to FetchContent-based Catch2 v3.11.0, now uses Catch2WithMain |
| test/matrixchain.cpp | Updated include to Catch2 v3 syntax and added stream operator for Fbitset to support Catch2 v3's stringification |
| test/testmain.cpp | Replaced custom main() with comments noting Catch2WithMain provides the runner |
| .github/workflows/ci.yml | Added new GitHub Actions CI workflow with matrix builds for Debug/Release configurations |
| .github/dependabot.yml | Added Dependabot configuration for GitHub Actions and git submodules |
| .circleci/config.yml | Removed obsolete CircleCI configuration |
| README.md | Updated CI badge to point to new GitHub Actions workflow |
| .gitmodules | Updated fbitset submodule URL to DrudgeCAS organization |
| .gitignore | Added build-*/ pattern to ignore additional build directories |
| .ycm_extra_conf.py | Removed obsolete YouCompleteMe configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| namespace fbitset { | ||
| template<int N> | ||
| inline std::ostream& operator<<(std::ostream& os, const Fbitset<N>& fs) { | ||
| os << "Fbitset<" << N << ">{count=" << fs.count() << "}"; | ||
| return os; | ||
| } | ||
| } |
There was a problem hiding this comment.
The stream insertion operator is being added to the fbitset namespace, which modifies a third-party library's namespace. This is generally not recommended as it can lead to ODR (One Definition Rule) violations if the fbitset library also defines this operator, or if multiple translation units define it differently.
A better approach would be to define this operator in an anonymous namespace or in a custom namespace specific to your tests. Alternatively, consider whether this could be contributed to the fbitset library itself if it's genuinely useful.
| add_executable(testmain | ||
| testmain.cpp | ||
| matrixchain.cpp | ||
| ) |
There was a problem hiding this comment.
The CMakeLists.txt no longer includes testmain.cpp in the add_executable call, but the file still exists in the repository. Since the file now only contains comments and Catch2::Catch2WithMain provides the main function, testmain.cpp should either be:
- Removed from the repository entirely (since it's not being used), or
- Added back to the sources list if it serves a purpose
The current state where the file exists but isn't compiled is inconsistent and may cause confusion.
Replaces CircleCI configuration with GitHub Actions workflow for CI/CD pipeline and updates the test infrastructure to use modern dependencies.
Changes
CI Migration
.github/workflows/ci.ymlwith matrix strategy testing Debug and Release builds in parallel.circleci/directory and added/build-*/to.gitignorecontents: readpermission following least-privilege principleTest Infrastructure Modernization
Dependency Management
.github/dependabot.ymlfor automatic updates of GitHub Actions workflows and git submodulesImplementation
The workflow runs on push/PR to master, checks out submodules recursively, installs cmake/g++, builds with CMake, and executes test suite. Replaces two separate CircleCI Docker jobs (tschijnmo/drudge:corebase, tschijnmo/gccpython:latest) with ubuntu-latest runner and matrix strategy.
Testing
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.