Skip to content

Run ReactCommon C++ unit tests in GitHub CI - #59802

Merged
meta-codesync[bot] merged 1 commit into
mainfrom
pr59802
Nov 6, 2026
Merged

meta-codesync[bot] merged 1 commit into
mainfrom
pr59802

Conversation

@huntie

@huntie huntie commented Nov 4, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

The C++ unit tests under packages/react-native/ReactCommon/**/tests run only inside Meta, under Buck. Public CI has no build for them, so a change that breaks one passes every GitHub check and fails only after import, as happened with #59686.

This adds a test_react_common_cxx job to Test All that builds and runs them on Linux, reusing the Fantom tester's CMake build, which already compiles every ReactCommon library with folly, Hermes and codegen.

Changes

  • Add private/react-native-fantom/tester/react-common-tests/: one GoogleTest executable per tests directory (38 today), linked against the libraries fantom_tester links. Each test case is registered with CTest through gtest_discover_tests, so it runs in its own process.
  • Add :private:react-native-fantom:testReactCommon, which builds them and runs ctest. GoogleTest 1.18.0 is downloaded like the Fantom tester's other dependencies.
  • Add the test_react_common_cxx job to test-all.yml, mirroring build_fantom_runner on the same container and runner. It adds no ccache steps: in this container compiles do not go through ccache, and build_fantom_runner on main reports 0 hits and 0 misses.

Notes

  • NOTE: Three Hermes debugger cases in jsinspector-modern/tests fail in OSS on macOS and Linux alike, and are excluded by name in the CMake file (see Test Plan).
  • The job installs clang 18 from apt.llvm.org, since the tests use C++20 features the image's clang 14 lacks: std::source_location and parenthesised aggregate initialisation.
  • Each case runs in its own process because some suites leave global state behind. In jsinspector-modern/tests, TracingTest fails when run after the other suites in one process, and passes on its own.
  • The tests compile with -Wno-error, since the header-only ReactCommon libraries export -Werror -Wpedantic and a few tests call deprecated APIs.
  • benchmarks/ directories are left out, as they need Google Benchmark.

Changelog: [Internal]

Test Plan

On macOS (Apple clang 21, CMake 3.30.5 from the Android SDK, JDK 17):

yarn install
./gradlew :private:react-native-fantom:testReactCommon

All 38 test executables build, and 1522 test cases pass, one of which skips itself (ReactInstanceTest.testRegistersRuntimeSchedulerAsEventLoopControl: this Hermes build has no ISetEventLoopControl).

Before the exclusions, 1525 cases ran and these 3 failed, with the same result on macOS and in this job on Linux:

  • JsiIntegrationHermesTest.ResolveColumnlessBreakpointAfterEval: Debugger.setBreakpointByUrl returns no locations.
  • JsiIntegrationHermesTest.ResolveColumnlessBreakpointAfterReload: the same.
  • JsiIntegrationHermesTest.TwoConnectionsDebuggerLifecycle: the second session gets no Debugger.scriptParsed after the first disables Debugger.

test_react_common_cxx on this PR: passed in 14m15s, with all 1522 tests passing. Running them takes 30 seconds of that; the rest is the build.

image file-bccb81bc0762aa22ac5f3c2165826db4 file-49c0f65500ec760461f11925dfbc4059

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 4, 2026
@huntie
huntie force-pushed the pr59802 branch 4 times, most recently from 4b836e7 to 1f264e7 Compare November 4, 2026 23:54
@huntie
huntie marked this pull request as ready for review November 5, 2026 11:44
@facebook-github-tools facebook-github-tools Bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Nov 5, 2026
@huntie huntie changed the title Run ReactCommon C++ unit tests in CI Run ReactCommon C++ unit tests in GitHub CI Nov 5, 2026

@cortinico cortinico left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code looks great ✅

@cortinico
cortinico enabled auto-merge (squash) November 5, 2026 12:17
@@ -0,0 +1,45 @@
name: Test ReactCommon C++

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can we have a reusable workflow rather than a reusable action? It's much easier to maintain. Also, if it is only used in test-all, can we just put the workflow there instead of splitting it in two places?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yeah it's semantically part of test-all, which is getting long at this point. For now I'll move this inline so we have less files to look at, at least (although I believe reusable actions are a fair pattern). The scope of this test suite is a single job in this workflow, IMO.

image

I'm certainly keen, after this, that we do another pass over all GHA files to simplify. But that goes beyond the initial scope here.

@meta-codesync

meta-codesync Bot commented Nov 5, 2026

Copy link
Copy Markdown

@cortinico has imported this pull request. If you are a Meta employee, you can view this in D129898681.

@huntie

huntie commented Nov 6, 2026

Copy link
Copy Markdown
Collaborator Author

@cipolleschi Can I get signal on what is failing internally here?

@vzaidman

vzaidman commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

@huntie

Seems like an internal issue. We have a few issues going on now. I'll let you know in a moment. I'm rebasing the PR.

EDIT: seems to be landing now.

@meta-codesync
meta-codesync Bot merged commit 6990b94 into main Nov 6, 2026
105 checks passed
@react-native-bot

Copy link
Copy Markdown
Collaborator

This pull request was successfully merged by @huntie in 6990b94

When will my fix make it into a release? | How to file a pick request?

@react-native-bot react-native-bot added the Merged This PR has been merged. label Nov 6, 2026
@huntie
huntie deleted the pr59802 branch November 6, 2026 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. p: Expo Partner: Expo Partner Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants