Aldriana / ShadowCraft-Engine

Calculations backend for ShadowCraft, a WoW theorycraft project.
GNU Lesser General Public License v3.0
37 stars 22 forks source link

Better Testing #11

Closed raconzor closed 13 years ago

raconzor commented 13 years ago

A real unit test framework would be good. Pyunit is something to look into. The current test.py isn't very robust - a complete regression suite would be ideal.

Dangerclose commented 13 years ago

I'm looking at some ways of adding in unit tests to help with this project. I looked at Pyunit and it looks like it could do it, but it's almost 10 years old and kinda feels it. A friend of mine who writes python code suggested looking at Fitnesse (http://fitnesse.org/FrontPage)

Dangerclose commented 13 years ago

An additional comment is that whatever testing is done, most probably each test should have an associated set of input variables and an output. These should be archived by Engine version in some way for traceability. So if something is noticed to be broken, it might be a little easier to go back and see when exactly it got broke.

julienp commented 13 years ago

The last time I looked at python testing, nose seemed like a popular choice http://somethingaboutorange.com/mrl/projects/nose/0.11.2/

julienp commented 13 years ago

Does anyone have any preferences or advice on how to structure the tests? I've added a test directory that mirrors the the actual app and wrote some simple tests just to see if it makes sense: https://github.com/julienp/ShadowCraft-Engine/blob/testing/tests/calcs_tests/init.py

I have no real experience writing unit tests, so I'm not entirely sure if I'm going about this in the right way, but unittest from the standard lib seems fine for doing this and it doesn't add any external dependencies to the project. I can run my tests from the project root directory with nosetest or individual testcases with python -m tests.calcs_tests.__init__ or similar. It shouldn't be a problem either to add a testsuite that runs all the tests without using nosetest if that's preferred.

Aldriana commented 13 years ago

There's sort of two things needed here.

The first is basically a way of exercising (most) code paths. As python as an interpreted language, there's no compiler to catch some of the stupid stuff that tends to creep into code. You can only find mistakes by trying to use the code and finding that it breaks. So for each object, the ability to hit each major code path would be nice. So, for instance, for glyphs: the test should set a few glyphs, and then check a) That a handful of glyphs that were set return True b) That a handful of glyphs that weren't set return False c) Feeding a bogus glyph name in returns an exception

Doesn't need to be fancy or in-depth, just needs to make sure the code paths work. I see this as mostly unit/regression testing that everyone should run before checking in to make sure they haven't broken anything.

2) A way of testing a full DPS module, to make sure its working like a DPS module. This might be able to stay as something akin to test.py, though perhaps moved or reorganized so we have one test file per DPS calculator. The general idea is that when writing an actual calculator, you need the ability to run it and check intermediate values and generally poke around and make sure they answers you're getting make sense. A lot of this consists of running it, then changing something a bit, then running it again, and seeing that it makes sense. For instance: I've just checked in a rough cut at a Mutilate DPS module. In addition to being somewhat incomplete, I guarantee that it has bugs. So I'm going to spend a lot of time enabling and disabling talents to make sure the model responds the way I expect it to. Some manner of framework that I can plug into to keep track of what changes I'm making and what the effect is seems like a good thing.

Dangerclose commented 13 years ago

The first step to achieving both of those goals is identifying 1) Each of the major code paths 2) All the inputs into each 'module'.

When I've done similar testing to this, we had a text file (or a.csv file) that listed each possible input variable in columns. Then each row provided a set of variables for each input variable (though some could be null). When we ran our test, we just read in each line of the text file and got values for each variable and used those to execute the test.

This made it quite easy to see 1) What variables had been tested and at what ranges 2) At a glance, what combination of variables have been run 3) Design of Experiment conditions where you can hold variables constant while adjusting others one at a time and see what variable has most impact.

raconzor commented 13 years ago

Further thought on test design - trace.py, or nosetest --with-coverage, can allow you to get code coverage results with options for line-by-line annotated results. That should help with determining what is needed to cover the existing code paths.

julienp commented 13 years ago

Here is what I got so far: https://github.com/julienp/ShadowCraft-Engine/tree/testing For each module of the app there is (or will be) a module_tests inside the tests folder that contains the corresponding unittests. For example the glyph tests: https://github.com/julienp/ShadowCraft-Engine/blob/testing/tests/objects_tests/rogue_tests/rogue_glyphs_tests.py

Currently you need to run them with nosetests (from the root directory), but I can easily make tests/init.py run them all at a later point.

This should basically cover the first point Aldriana outlined.

If this basic structure is ok, I'll go on adding tests and hopefully cover most of the classes/methods.

The tests for most methods are pretty simple, but for something like get_dps from the calculator I assume checking if the method finishes and gives a reasonable result (positive number, maybe above 10000 and lower than 30000 or similar) should be enough to see if it basically works.

julienp commented 13 years ago

An update on testing, I'm slowly adding tests for all codepaths and got pretty good coverage so far (coverage.py is great!). There's things that are only tested implicitly because other tested functions call them, but the test suite hits almost every line of code outside of the AldrianasRogueDamageCalculator class.

The test suite is in the tests folder in my testing branch: https://github.com/julienp/ShadowCraft-Engine/tree/testing

You can run it with python tests/runtests.py from the project root directory. coverage run tests/runtests.py followed by coverage report or coverage html (for nice HTML output) to view the code coverage.

A lot of the tests are very simple and don't tell you much about how accurate the result is, but they do catch typos, type errors and the like.

I'd be happy about feedback, everything is up for discussion and there's lots of tests that can be extended, but for now my focus has been on covering the entire codebase.

Aldriana commented 13 years ago

Is this in a state where I should pull it into the main branch? I've held off on doing so because the lack of pull requests made me think you thought it wasn't quite ready, but if it's in a generally usable state it'd probably be good to include it at some point.

julienp commented 13 years ago

Yea, I think it's in a reasonable and usable state now and there shouldn't be too many changes to the core classes and methods, so you won't have to change tests a lot. Biggest incoming change that I can think of will be when the asserts get changed to exceptions.

I'll post a pull request so people can review it, I'm sure there's some silly things in there I missed.

Aldriana commented 13 years ago

Now that we have real testing, I'm going to start stripping the asserts and stuff out of tests.py to make it easier to change around gear for debugging purposes. If you have concerns about this, let me know.

Aldriana commented 13 years ago

We seem to have a decent test framework in place, so this is largely done. We should keep adding and updating tests, of course, but I don't see that we need a separate issue to track it. Closing.