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.

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

Add tests for effects #6204

Merged
merged 14 commits into from
Jan 1, 2024
Merged

Add tests for effects #6204

merged 14 commits into from
Jan 1, 2024

Conversation

Pikachu920
Copy link
Member

Description

This is a PR to add tests for as many effects as I can.


Target Minecraft Versions: any
Requirements: N/A
Related Issues: #6158

@@ -40,7 +40,7 @@ dependencies {
implementation fileTree(dir: 'lib', include: '*.jar')

testShadow group: 'junit', name: 'junit', version: '4.13.2'
testShadow group: 'org.easymock', name: 'easymock', version: '5.2.0'
testShadow group: 'org.easymock', name: 'easymock', version: '5.0.1'
Copy link
Member Author

Choose a reason for hiding this comment

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

it seems like 5.2.0 has a bug. on v5.0.1, the action bar test passes. however, when running this exact same code in v5.2.0, the action bar test encounters the exception described in easymock/easymock#356. that issue was apparently fixed in 5.2.0 but i wonder if it wasn't a complete fix. it could definitely be user error too though. i will probably followup on that issue and see what the maintainers think.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

@sovdeeth sovdeeth left a comment

Choose a reason for hiding this comment

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

I don't think I know enough about EasyMock to comment on that code with authority, but everything looks alright from a glance. This seems like a very useful tool for player-related testing!
I may come back and actually approve once I know a bit more about how easymock works.

@sovdeeth sovdeeth added the enhancement Feature request, an issue about something that could be improved, or a PR improving something. label Nov 26, 2023
@Pikachu920
Copy link
Member Author

I don't think I know enough about EasyMock to comment on that code with authority, but everything looks alright from a glance. This seems like a very useful tool for player-related testing! I may come back and actually approve once I know a bit more about how easymock works.

it's really pretty simple! i don't know how it's actually implemented, but it's basically like a proxy that keeps track of it's method calls. You use expect to tell it what calls you are expecting, then you use replay to say "start listening for the expected calls", then you use verify to say "did the expected calls occur?". it can also do more complex things like ensuring the arguments match some criteria and stuff like that.

Copy link
Member

@Moderocky Moderocky left a comment

Choose a reason for hiding this comment

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

Nice work!

@Pikachu920 Pikachu920 marked this pull request as ready for review December 30, 2023 23:04
@sovdeeth sovdeeth added the 2.8 Targeting a 2.8.X version release label Dec 30, 2023
@sovdeeth sovdeeth added the feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version. label Dec 31, 2023
@sovdeeth sovdeeth merged commit 5843ec8 into dev/feature Jan 1, 2024
5 checks passed
@sovdeeth sovdeeth deleted the feature/effect-tests-1 branch January 1, 2024 00:53
APickledWalrus added a commit that referenced this pull request Apr 30, 2024
We cannot use EasyMock on Java 21 as we must stay on an older version (see #6204 (comment))
Java 21 will be supported for regular testing. A 1.20.4 environment remains for Java 17
Moderocky added a commit that referenced this pull request May 1, 2024
* Internal support for aliases item components

* Update aliases submodule

It now targets to the 1.20.5 support branch

* Update isRunningMethod check

* Add ability to opt out of deduplication

This is useful for items that have NBT that is not represented by an ItemMeta implementation (e.g. block entity data)

* Update skript-aliases

* Update skript-aliases

* Update skript-aliases

* Update Gradle to 8.7

* Update Paper to 1.20.6

Also updates Java from 17 to 21

* Fix latest env for testing

* Fix AreaEffectCloudApplyEvent event value

* Fix vehicle dismount events

* Fix potions

* Get visual effects building

Removed ones do not currently work on new versions...

* Fix unused AbstractList implementation

* Fix remaining Java 17 references

* Fix 1.17 testing

* Add missing PotionDataUtils check

* Add missing lang entries

* Rework Test Environments

We cannot use EasyMock on Java 21 as we must stay on an older version (see #6204 (comment))
Java 21 will be supported for regular testing. A 1.20.4 environment remains for Java 17

* Update workflow names

---------

Co-authored-by: Moderocky <admin@moderocky.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.8 Targeting a 2.8.X version release enhancement Feature request, an issue about something that could be improved, or a PR improving something. feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants