-
Notifications
You must be signed in to change notification settings - Fork 49
Move GTest: src/beanmachine/*/tests/ -> tests/*/ #1742
base: main
Are you sure you want to change the base?
Conversation
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. |
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. |
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? |
@feynmanliang has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
It should be a matter of linking against 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. |
@ntfrgl do you mind rebasing 😓 ? |
@ntfrgl has updated the pull request. You must reimport the pull request before landing. |
@feynmanliang has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
This PR is testing if we can merge PRs into `main`. The only thing that has changed is the `README.md` file. **No** content was changed, only formatting, which was done with `mdformat` with the standard line length defined by `black`, and standardization of the markdown.
The commit modifies the GitHub actions to ensure the needed directory for `vcpkg` exists before running the `vcpkg integrate install` command.
The `forward` method in the `Regression` class in module `tests/ppl/experimental/gp/inference_test.py` was failing due to an attribute error. This commit attempts to fix that.
- reflect test relocation in e1e1eca - enable parallel compilation of Pybind11 extensions
- multi-version Actions: add latest release - single-version Actions: move to oldest actively developed release
- reuse installed dependencies: - from Conda: Boost, Eigen, GTest - from Vcpkg: range-v3 - execute GTest suite via CMake's test runner
Hi Boyan, I'm no longer at Meta but my understanding is that beanmachine is no more. Perhaps @horizon-blue can advise? |
@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. |
Oooh.... Does |
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
src/
totests/
. At present, this does not affectminibmg/tests/
.[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
Checklist