Add test for org.bukkit.craftbukkit.entity.CraftMinecart#minecartEntityTypeToMaterial

[Abelkrijgtalles]

Hi, I hope you're having a wonderful day.
I've added a test for org.bukkit.craftbukkit.entity.CraftMinecart#minecartEntityTypeToMaterial, as requested in #11511.
In the current approach I've hardcoded the Minecart types, as is done in the function.

[lynxplay]

The whole point of the test is to ensure that the hardcoded values in the function are still correct.
Testing this with another set of hardcoded function is useless.
Iterate the item registry, find MinecartItems, reflectively access their entity type, ensure the static method correctly matches that.

[Abelkrijgtalles]

Alright, I will try to fix it to use the Item registry

[lynxplay]

Also merge this into the respective patch that introduces the method under test.
No need for a new patch here.

[Abelkrijgtalles]

I've fixed it to not be hard-coded, except for EntityType.SPAWNER_MINECART, as that doesn't have a corresponding item, and the function just returns a normal minecart. I've used a string based system, as I couldn't find any other way to do it. The current approach gets the short string of the entitytype (like chest_minecart), and gets the item with the corresponding ResourceLocation

[lynxplay]

Thanks!
You'll need to annotate the class with @AllFeatures in order for the test suite to pick it up and run correctly.

[Abelkrijgtalles]

I'll probably also make a PR for changing it to a switch statement, as that's a better choice then 7 if/else statements.

[Abelkrijgtalles]

Thanks! You'll need to annotate the class with @AllFeatures in order for the test suite to pick it up and run correctly.

Will do

[lynxplay]

EntityType fields are not constants, you won't find much luck in making that a switch statement.

[Abelkrijgtalles]

I've added it, so it should be good to go.

[Abelkrijgtalles]

I've removed the empty lines. If it's to aggressive, just let me know.

[Leguan16]

Ok, that's not what I meant lol. Just revert it and forget what I said please.

[Abelkrijgtalles]

Alright.

[Abelkrijgtalles]

Hi, I was just wondering, are there any specific things holding the merge back?

[lynxplay]

The fact that no developer had time to review this yet.
Give it time, someone will get it merged soon enough.

[Abelkrijgtalles]

Alright, thanks for the update

[lynxplay]

Can you reopen the PR to master please :)
I'll try to review it tonight and get that merged.