Draft
Conversation
Member
Author
|
Pinging @shartte, the original author of |
MattSturgeon
force-pushed
the
test/init
branch
from
a26b8df to
bc81682
Compare
|
You can |
MattSturgeon
added
enhancement
MattSturgeon
force-pushed
the
test/init
branch
from
dda0856 to
38f1871
Compare
Since 1.17, Minecraft checks whether `Bootstrap` has been called, when any of its registries static constructor runs. This means we have to explicitly bootstrap test cases that use any of the registries. Added an annotation `@BootstrapMinecraft` to support this.
(cherry picked from commit 05b9860)
MattSturgeon
force-pushed
the
test/init
branch
from
5a43894 to
e6fa497
Compare
MattSturgeon
commented
Feb 1, 2024
| - '1.18' | ||
| - '1.19' | ||
| pull_request: | ||
| branches: *branches |
Member
Author
There was a problem hiding this comment.
The workflow is not valid. .github/workflows/main.yml:
Anchors are not currently supported. Remove the anchor 'branches'
I guess GitHub doesn't like yaml anchors...
Suggested change
| branches: *branches | |
| branches: | |
| - 'main' | |
| - '1.16' | |
| - '1.17' | |
| - '1.18' | |
| - '1.19' |
Member
Author
|
Putting this comment from the Architectury discord here for my own reference...
|
Member
Author
|
Looks like neoforged/FancyModLoader#54 will provide a Neoforge equivalent of |
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
2 participants
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.
Beginning work on unit tests, opening a draft PR for early feedback.
The key breakthrough was calling
Bootstrap.bootStrap()before running tests; without bootstrapping minecraft, registries (etc) are not initialized and essentially no MC classes can be constructed. This apparently affects 1.17 & newer.My solution is based on this GPL3 code, see comments below.
The goal of this PR is not to provide full test coverage, nor is this PR attempting to add integration tests with minecraft dependencies. That can all come in the future.
Instead this PR aims to enable basic unit testing functionality and provide one or two examples as a starting point.
Integration tests will likely require a
testmod/run configured in loom, since the game will actually have to run to test integration. We might be able to do something with Mojang'sGameTestsystem 🤔. That's all out of scope for this PR though.JUnit 5 is chosen as the testing toolkit to run the tests. Upstream docs.
Mockito is chosen as a mocking framework to mock class dependencies. Upstream docs.
fabric-loader-junit tells JUnit to use fabric's
Knotclassloader. This enables things like mixin & Environment stripping.See also
Writing good tests, in particular: don't mock a type you don't own; a long term goal should be to reduce hard dependencies on mc classes & also write integration tests.
AssertJ (alternative assertions syntax we could use instead).
Fixes #151