Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Game Test API #226

Merged
merged 17 commits into from Feb 7, 2023
Merged

Game Test API #226

merged 17 commits into from Feb 7, 2023

Conversation

LambdAurora
Copy link
Member

One of the most important addition for stronger testing!

This API is, well, the game test API, which allows mods to use the Minecraft test automation framework (keynote about it by Henrik Kniberg).

This API introduces a new entrypoint quilt:game_test of classes implementing QuiltGameTest. Any method annotated with GameTest in that class will be registered as a test, and you can implement a method from QuiltGameTest to programmatically register more test classes.

The API is partially a port from Fabric API's game test API, but extended to be more comfortable to use, and more powerful!

As you can see there's also some game tests implemented for Block Content Registries to show how it can be used (and its benefits).

@LambdAurora LambdAurora added new: module A pull request which adds a new module library: core Related to the core library. t: new api This adds a new API. s: tested This pull request has been tested and confirmed as working. labels Dec 4, 2022
@LambdAurora LambdAurora added this to the Initial release milestone Dec 4, 2022
@LambdAurora LambdAurora requested a review from a team December 4, 2022 13:07
@LambdAurora LambdAurora requested a review from a team as a code owner December 4, 2022 13:07
EnnuiL
EnnuiL suggested changes Dec 7, 2022
View reviewed changes
Copy link
Contributor

@EnnuiL EnnuiL left a comment

Choose a reason for hiding this comment

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

oh wow, this PR looks great!
I only have few nitpicks

build.gradle Outdated Show resolved Hide resolved
gradle.properties Outdated Show resolved Hide resolved
...k_content_registry/src/testmod/java/org/quiltmc/qsl/block/test/BlockContentRegistryTest.java Outdated Show resolved Hide resolved
library/core/game_test/src/main/java/org/quiltmc/qsl/game_test/api/package-info.java Outdated Show resolved Hide resolved
@LambdAurora
Apply suggestions from code review
b37ac4e 
Co-authored-by: Ennui Langeweile <85590273+EnnuiL@users.noreply.github.com>
@TheGlitch76 TheGlitch76 self-requested a review December 7, 2022 20:22
TheGlitch76
TheGlitch76 requested changes Dec 7, 2022
View reviewed changes
Copy link
Member

@TheGlitch76 TheGlitch76 left a comment

Choose a reason for hiding this comment

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

The documentation in the package-info is really nice! I provided some really picky corrections, but the structure is really really good.

library/core/game_test/src/main/java/org/quiltmc/qsl/game_test/api/package-info.java Outdated Show resolved Hide resolved
library/core/game_test/src/main/java/org/quiltmc/qsl/game_test/api/package-info.java Outdated Show resolved Hide resolved
library/core/game_test/src/main/java/org/quiltmc/qsl/game_test/api/package-info.java Outdated Show resolved Hide resolved
library/core/game_test/src/main/java/org/quiltmc/qsl/game_test/api/package-info.java Outdated Show resolved Hide resolved
library/core/game_test/src/main/java/org/quiltmc/qsl/game_test/api/package-info.java Outdated Show resolved Hide resolved
library/core/game_test/src/main/java/org/quiltmc/qsl/game_test/api/package-info.java Outdated Show resolved Hide resolved
library/core/game_test/src/main/java/org/quiltmc/qsl/game_test/api/package-info.java Outdated Show resolved Hide resolved
library/core/game_test/src/main/java/org/quiltmc/qsl/game_test/api/package-info.java Outdated Show resolved Hide resolved
library/core/game_test/src/main/java/org/quiltmc/qsl/game_test/api/package-info.java Outdated Show resolved Hide resolved
library/core/game_test/src/main/java/org/quiltmc/qsl/game_test/api/package-info.java Outdated Show resolved Hide resolved
williambl
williambl reviewed Dec 8, 2022
View reviewed changes
Copy link
Contributor

@williambl williambl left a comment

Choose a reason for hiding this comment

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

Looks good! I'll give it a proper look/test in my IDE later :D

library/core/game_test/src/main/java/org/quiltmc/qsl/game_test/mixin/server/MainMixin.java Outdated Show resolved Hide resolved
@EnnuiL
Copy link
Contributor

EnnuiL commented Jan 19, 2023

Hm, as a precaution, we should probably consider merging this GameTest mappings PR before doing any further push for release

@LambdAurora
Copy link
Member Author

Hm, as a precaution, we should probably consider merging this GameTest mappings PR before doing any further push for release

that mappings PR doesn't target the version this PR targets anymore :)

@EnnuiL
Copy link
Contributor

EnnuiL commented Jan 19, 2023

Hm, as a precaution, we should probably consider merging this GameTest mappings PR before doing any further push for release

that mappings PR doesn't target the version this PR targets anymore :)

Not if we downgrade it back!

@EnnuiL
Copy link
Contributor

EnnuiL commented Jan 22, 2023

Hm, as a precaution, we should probably consider merging this GameTest mappings PR before doing any further push for release

that mappings PR doesn't target the version this PR targets anymore :)

Not if we downgrade it back!

And with some poking? My dreams became true! :P

But yeah, the PR has been merged on the 22w03a branch and backported to the 1.19.3 one on QM 1.19.3-build.18

@LambdAurora LambdAurora requested a review from a team January 26, 2023 23:48
EnnuiL
EnnuiL approved these changes Jan 27, 2023
View reviewed changes
Copy link
Contributor

@EnnuiL EnnuiL left a comment

Choose a reason for hiding this comment

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

The only change that I'd make is adding the "You agree to the EULA by running these tests" message, but yeah, it can be done after merge

@EnnuiL EnnuiL merged commit 932c5c9 into QuiltMC:1.19.3 Feb 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@williambl williambl williambl left review comments

@Leo40Git Leo40Git Leo40Git approved these changes

@OroArmor OroArmor OroArmor approved these changes

@Mysterious-Dev Mysterious-Dev Mysterious-Dev approved these changes

@EnnuiL EnnuiL EnnuiL approved these changes

@TheGlitch76 TheGlitch76 Awaiting requested review from TheGlitch76

Assignees
No one assigned
Labels
final-comment-period library: core Related to the core library. new: module A pull request which adds a new module s: tested This pull request has been tested and confirmed as working. t: new api This adds a new API.
Projects
None yet
Milestone
Initial release
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@LambdAurora @EnnuiL @williambl @Leo40Git @OroArmor @TheGlitch76 @Mysterious-Dev