louis-e / arnis

Arnis - Generate cities from real life in Minecraft
GNU General Public License v3.0
260 stars 18 forks source link

Add automated testing #7

Closed EdwardWeir13579 closed 1 year ago

EdwardWeir13579 commented 1 year ago

Adding automated tests to some of the code.

pytest is a better testing package that the default one in python, it is also what the other project you have referenced (bresenham) already uses. I have added it to the requirements and instructions to the readme. There are a few ways of structuring tests within the project, but the one I tend to use is to make a mirror of the main code, and prefix every file and folder with "test_", then write the tests in the matching file. I have done this for "test_bresenham.py".

The tests I have added are the ones from the package you linked in the function being tested. I have formatted them, updated the variable names to match yours, and improved them slightly, but it is still the same numbers being tested.

If you want you can just reference that other project through requirements.txt, and not have it in your own code base. However there is nothing wrong with the way you have it, particularly since it is only one function. But if you do copy anything more in the future you will want to copy the tests too, assuming the source project has them.

louis-e commented 1 year ago

Sounds interesting, thanks! I'm currently very busy with work, I'll take a look into this in the next few days!🚀

callumfrance commented 1 year ago

very good idea, you can also extend the github action (.github/workflows/python-app.yml) to run the test in the browser.

Adding something like the following to the end of the file -

      - name: Run unit tests
        run: |
          python -m pytest
EdwardWeir13579 commented 1 year ago

Yeah that is reasonable. Do you want to make another PR with that change and @louis-e can take a look after this one is merged?

EdwardWeir13579 commented 1 year ago

@callumfrance I added that config to the file.

@louis-e I have resolved the conflicts.

louis-e commented 1 year ago

Hi there! Sorry for the late reply, I've been very busy recently. You guys are awesome! Thanks for the effort, I learned a lot so far with the help from you all. Will merge it!