twoolie / NBT

Python Parser/Writer for the NBT file format, and it's container the RegionFile.
MIT License
362 stars 74 forks source link

Update tests for example scripts #113

Open macfreek opened 5 years ago

macfreek commented 5 years ago

Currently, most test are done on "Sample World" which is an McRegion file. Ideally, we should have a "McRegion Sample World" (this one), a "Anvil Sample World" and a "Flattened Sample World", and run the test scripts -were applicable- on all worlds.

Also, we need to check if all example scripts are tested.

I'm not sure how to convert the Sample World in a way that does not change it other than syntax conversion (e.g. prevent the client from generating new chunks, or moving entities.). And if we can not convert it that way, how to update the test suites.

mfld-fr commented 5 years ago

Suggestion : throw away the current sample worlds (rely on a deprecated GitHub feature), modify the test script to download various MC server versions from the Mojang official web site (server because no user account needed), use them to generate various versions of the sample world with the same seed, then run the tests on them.

mfld-fr commented 5 years ago

I just made a basic test :

Size of the generated overland is small but large enough to run tests:

world

Older versions could be found here : https://mcversions.net/

macfreek commented 5 years ago

What deprecated Github feature do you refer to?

I'm cautiously optimistic about your idea. Let's see if I can the implications straight.

Your proposal makes this library more Minecraft specific. Originally nbt.py is generic to the NBT file format, which could have been used by other programs. However, since it is not picked up elsewhere, I see little point in keeping that separation.

More fundamentally, this will mean that tests are run to check if scripts work with recent Minecraft versions. That means you change what you test. Previously, the tests are designed to check if new commits break existing code. They verify that whatever commit is made, it still works on the same old test file(s). In your proposal, the tests are designed to check if the code still works for new Minecraft versions. So rather than testing the NBT library, it tests compatibility with recent Minecraft versions.

I think that this is a useful thing to test against. I am not sure if that is what e.g. Travis CI/CD tests are designed for. Travis is designed to check if a commit breaks existing code.

Is my thinking correct, and if so, what is your vision on this?

macfreek commented 5 years ago

I've been pondering, and yes, I think your proposal is good. We need both type of checks.

Running the example scripts used to be a way to ensure the public API of the library did really not change. Well, it's better to to that type of verification in nbttests.py and andregiontests.py (to be augmented with chunktests.py and worldtests.py). So we no longer need to run the examples for that. Using the examples to verify compaitibility with new Minecraft versions really seems like a good check to ensure this library remains up-to-date and interesting for users.

Whether we should include these tests in Travis, I don't know. I'm inclined to say yes, but make it clear to future developers what is being tested (and perhaps there is no need to run it against all Python versions.)

mfld-fr commented 5 years ago

What deprecated Github feature do you refer to?

https://github.blog/2012-12-12-goodbye-uploads/

More fundamentally, this will mean that tests are run to check if scripts work with recent Minecraft versions.

You can pick any significant MC version listed on https://mcversions.net/ (basically a collection of links to the Mojang archives) if you want to maintain backward compatibility with older world formats.

Is my thinking correct, and if so, what is your vision on this?

I have no vision, just some user needs with "have fun" as the main goal:

Currently, the library fulfills that needs. So I am happy with it. That's for sure a very short sight, that won't help you as maintainer to make the good choices, but that shortness saves my time 😀 !