Move GTest: src/beanmachine/*/tests/ -> tests/*/

[ntfrgl]

Motivation

In preparation for a refactoring of the Bean Machine test suite, #1674 and #1711 moved the Pytest modules (BM/Python) into a dedicated tests/ directory. This is now followed by a corresponding move of the GTest suite (BMG/C++).

In addition, a new CMake build of BMG defines an entry point for the GTest suite to be executed from the GitHub Actions CI --- previously, this was only configured in the internal build system.

The choice of CMake as the testing build system is based on its official support both by GTest and by Vcpkg. Separate linking (BMG+Pybind11, BMG+BMG_test+GTest) is preferable over full linking (BMG+Pybind11+BMG_test+GTest), because it avoids adding new build requirements (i.e., CMake) to the Python package and allows for testing BMG/C++ independently of BM/Python.

Changes proposed

• C++ tests are relocated from src/ to tests/. At present, this does not affect minibmg/tests/.
• The Python package definition is updated, and compilation of the Pybind11 extension is parallelised [1].
• A CMake build of BMG with GTest integration [2] is defined, independently of the Pybind11 integration.
• The new GTest entry point is added to the GitHub Actions.

[1] https://pybind11.readthedocs.io/en/latest/compiling.html#building-with-setuptools [2] https://google.github.io/googletest/quickstart-cmake.html

Test Plan

Add the GTest suite to the GitHub Actions workflow "Tests".

Types of changes

• [x] Docs change / refactoring / dependency upgrade
• [ ] Bug fix (non-breaking change which fixes an issue)
• [x] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• [x] My code follows the code style of this project.
• [ ] My change requires a change to the documentation.
• [ ] I have updated the documentation accordingly.
• [x] I have read the CONTRIBUTING document.
• [x] I have added tests to cover my changes.
• [ ] All new and existing tests passed.
• [x] The title of my pull request is a short description of the requested changes.

[feynmanliang]

Are we planning to refactor C++ tests as well? It makes sense to re-use Python fixtures in python tests, but I'm less clear about opportunities for re-use within BMG.

[ntfrgl]

The Pytest fixtures are just the first, and most pressing, step of a larger refactoring effort across the BM and BMG test suites, which will increase their modularity as well as coverage of the coupling between front-end (BM) and back-ends (BM, BMG etc.). So the answer to your first question is yes, while the explanation will only unfold over a sequence of PRs which should be independently justifiable. In particular, the change proposed here was already discussed with the BMG team.

[horizon-blue]

Thanks for the 2nd test reorganization. Do we have a plan to create an entry point to run GTest as part of this PR to enable C++ testing on external platforms?

[facebook-github-bot]

@feynmanliang has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[ntfrgl]

It should be a matter of linking against gtest_main, or, in case there are some fixtures which are currently not visible externally, adding them to a custom main() function which defines them before calling RUN_ALL_TESTS(). https://google.github.io/googletest/primer.html#invoking-the-tests

Since this change will need to be carefully compared against the internal behaviour, I'm assuming that it will be easiest for @rodrigodesalvobraz to handle. But please let me know if I can help with that.

[feynmanliang]

@ntfrgl do you mind rebasing 😓 ?

[facebook-github-bot]

@ntfrgl has updated the pull request. You must reimport the pull request before landing.

[facebook-github-bot]

@feynmanliang has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[feynmanliang]

Hi Boyan, I'm no longer at Meta but my understanding is that beanmachine is no more. Perhaps @horizon-blue can advise?

[horizon-blue]

@feynmanliang We've had some discussions around the future of Bean Machine with OpenTeams. Even though the Meta team that officially supports it was gone, there's still some interests to keep BM alive for the community. So we detached BM from Meta's codebase and turned it into a GitHub-only project. Now people should be able to continue maintaining BM just like any other GitHub repo, and Meta no longer need to be involved to approve and merge PR.

[feynmanliang]

Oooh.... Does beanmachine need an OSS steward (e.g. Vercel and Svelte/React)? We (Aileen ) might be able to pick it up...