Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds an integration-style PHPUnit testing setup for the DiceForge codebase by introducing a real MySQL/MariaDB-backed test DB fixture, plus supporting framework stubs, a thin DB wrapper for testability, and CI wiring to run formatting + tests.
Changes:
- Added a test database bootstrap/fixture (real MySQL via
mysqli) and updated unit tests to exercise real SQL writes/reads. - Introduced injectable abstractions for DB access (
Db) and randomness (RandomProvider) to improve testability. - Added GitHub Actions CI job with MySQL service, plus PHPStan configuration/custom rule plumbing.
Reviewed changes
Copilot reviewed 28 out of 30 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Support/DbFixture.php | Creates/destroys a fresh test schema from dbmodel.sql and provides helpers to seed players. |
| tests/Stubs/PhpstanStubs.php | IDE-only stubs for PHPStan classes/types. |
| tests/Stubs/BgaFrameworkStubs.php | Large in-memory stubs for BGA framework classes/functions used during tests. |
| tests/setup-test-db.sh | Local one-time setup script for creating a bga_test MySQL/MariaDB database/user. |
| tests/ResourceChoiceHelperTest.php | Converted test to use real DB fixture instead of mocking DB calls. |
| tests/README.md | Documents prerequisites and how to run tests locally. |
| tests/Game/GameBeginnerTest.php | Adds a game-setup test that asserts players are persisted via the new repository/fixture. |
| tests/Game/fixtures/PlayerProvider.php | Provides sample player sets for tests. |
| tests/Game/doubles/TestGame.php | Exposes protected Game methods for testing. |
| tests/bootstrap.php | Loads framework stubs and adds autoloaders for game/test namespaces. |
| phpstan.neon | Registers custom PHPStan rule and updates stub scan file path. |
| phpstan-custom-rules.neon | Adds separate config for running custom rules. |
| modules/php/tokens.php | Refactors DB usage to go through injected Db wrapper. |
| modules/php/ResourceChoiceHelper.php | Refactors to use injected Db wrapper instead of a bespoke test interface. |
| modules/php/RandomProvider.php | Adds a randomness interface in the root game namespace. |
| modules/php/random/RandomProvider.php | Adds a randomness interface in Bga\Games\diceforge\Random (used by Game). |
| modules/php/random/BgaRandomProvider.php | Production random provider wrapping bga_rand(). |
| modules/php/PHPStan/EnforceDbWrapperRule.php | Adds a custom PHPStan rule to discourage self::DbQuery()-style usage. |
| modules/php/Game.php | Injects Db + RandomProvider and refactors DB calls to use $this->db. |
| modules/php/Framework/Orm/Id.php | ORM attribute for primary key marker. |
| modules/php/Framework/Orm/Entity.php | ORM attribute for table mapping. |
| modules/php/Framework/Orm/Column.php | ORM attribute for column mapping. |
| modules/php/Framework/Db/TableDb.php | Production Db implementation that forwards to Table static DB APIs. |
| modules/php/Framework/Db/Repository.php | Generic attribute-driven repository for find/findAll/save. |
| modules/php/Framework/Db/MysqliDb.php | Test Db implementation backed by real mysqli. |
| modules/php/Framework/Db/Db.php | Db interface used for injection/mocking and decoupling from Table statics. |
| modules/php/Entities/Player.php | New Player entity annotated for the generic repository. |
| .vscode/sftp.json.example | Adds an example SFTP config for BGA Studio deployments. |
| .gitignore | Allows committing .vscode/sftp.json.example while ignoring other .vscode/*. |
| .github/workflows/ci.yml | Consolidates CI and adds MySQL service + phpunit run. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…hen running tests Enforce that the static Db methods are never called directly but always through the wrapper
Stop enforcing db wrapper usage on CI, it takes quite a bit of time to run on CI. There's not much value to that as it's unlikely to happen, especially as we write tests against a real DB. Additionally, it is currently set up in my git pre-commit hook. So we should be safe.
Also add instructions to install a mysql server locally. The objective is to be able to write non-regression integration tests for the Game class by injecting a Db instance with a test database.
This will allow to inject a deterministic provider for intergration test, which will be used to play hardcoded test games with predictable data
It tests that a game can be created with two players, and that the initial state of the game is correct. For that purpose, the Player entity has been created, backed by a small custom ORM. Reorganize the project structure
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The main focus of this PR was to add a test for the main Game file.
To achieve that, I've needed to abstract out the direct calls to the DB that Game performed, so that it goes through an object that can be injected in the constructor. That object is of a Db interface, the default implementation is the one that calls Table:: methods directly, and another implementation backed by a local mysql DB can be injected in tests.
It required to install mysql in the CI workflow so that the test could run on CI.
I've also abstracted out the Randomizer in a similar way - I'll need that in the future to return pre-determined numbers instead of random numbers, in order to run predetermined games.
The test is very basic so far: it calls the Game's initial setup methods with hopefully a realistic payload from the BGA framework (I understand it is closed source so I inferred as well as I could), and it then makes sure the expected player have been created in the DB.
In order to achieve that in a clean way, I've introduced a very tiny ORM (which is meant to be extended) so that we can CRUD on Player (and future entities thanks to an entity agnostic Repository class) in a modern way (e.g. without having to write RAW SQL every time we want to CRUD 😅 )
Up next: extending the test case to cover an entire game, then entire games with various options, etc. The ideal long-term goal is to cover 100% of the Game.php code, reproducing know bugs in the process, which would allow refactoring and fixing bugs with a low risk of introducing regressions.
As I can't code (yet) in PHP, this stuff was written by GitHub Copilot, using Claude Sonnet 4.6.