AdamsLair / duality

a 2D Game Development Framework
https://adamslair.github.io/duality
MIT License
1.4k stars 290 forks source link

Write More Unit Tests #114

Closed AdamsLair closed 8 years ago

AdamsLair commented 10 years ago

Duality already incorporates automated testing into its development and deployment cycle. However, these tests barely cover even half of Duality and while full coverage may not be necessary, a little bit more than now can't be too bad.

Testing Guidelines:

Things to think about how to test:

ghost commented 10 years ago

Hi! I would like to contribute to solve this issue. I have the project locally, in my machine. I have done unitary test in the past, in Java. This is very similar.

The thing is that in the project actually there is a folder named Unit Test and some unitary tests there. There is a way to create a "Unit Test Folder" in Visual Studio and i don't know what to do, if add mi PathHelperTest.cs to the existing folder or create a "official" folder for the new test.

In the image the folder i've created is the last named "DualityTest"

image

Thanks for your time!

AdamsLair commented 10 years ago

Duality isn't using the Unit Testing framework provided by Visual Studio, but instead relies on NUnit, so creating a new Test Project is not the way to go. I'd suggest getting to know both NUnit and Duality itself a little better first :) There are also a lot of easier ways to contribute that are especially suitable for new users.

ghost commented 10 years ago

Hi Adam! I would to contribute making some code if it's posible. I already playing and using Duality for make a very simple game :).

By the way, i'll check out NUnit.

Thanks!

mattm401 commented 9 years ago

Are there known, or suspected, issues with the classes you mentioned above or do you just need unit tests for them for test adequacy?

AdamsLair commented 9 years ago

There are no known or suspected issues, it's just that there are no unit tests for them yet. It's not urgent, but at the same time, you never know what hidden bugs become visible in extensive testing. Mostly a routine task.

BraveSirAndrew commented 9 years ago

For me, it's less about hidden bugs and more about maintainability. We've made quite a few changes to our fork of Duality now. Without tests, how do we know that the changes we've made haven't broken something else? It makes us hesitant to make big changes, and it would be a nice cozy safety blanket for you Adam in accepting pull requests, knowing that they pass all the things. I'd probably raise the priority of this as Duality becomes more popular, because the lack of good tests will most likely slow down its development. Soapbox rant off:)

On the question of testing editor functionality, I've been meaning to mention this one for a while now but have you looked at using some version of the MVP pattern for this? It would mean a lot of refactoring of existing editor code to extract the logic of things from the WinForms stuff, but it's pretty much the standard way of making WinForms/UI stuff testable, and it works well for us with our custom editors.

AdamsLair commented 9 years ago

For me, it's less about hidden bugs and more about maintainability. We've made quite a few changes to our fork of Duality now. Without tests, how do we know that the changes we've made haven't broken something else? It makes us hesitant to make big changes, and it would be a nice cozy safety blanket for you Adam in accepting pull requests, knowing that they pass all the things. I'd probably raise the priority of this as Duality becomes more popular, because the lack of good tests will most likely slow down its development. Soapbox rant off:)

Good point! :) What he said. It's actually a valid maintainability aspect for me too, but probably less severe than for others and custom forks of Duality.

One other thing to keep in mind is to have an eye on actual test maintainability: In the current state of the Duality framework, Tests shouldn't focus too much on small details and keep to a broader functionality scope, because I can't afford to have to rewrite 50 tests each time I change something in the core. However, they shouldn't be too broad as well. It's all a matter of reasonable scope.

On the question of testing editor functionality, I've been meaning to mention this one for a while now but have you looked at using some version of the MVP pattern for this? It would mean a lot of refactoring of existing editor code to extract the logic of things from the WinForms stuff, but it's pretty much the standard way of making WinForms/UI stuff testable, and it works well for us with our custom editors.

Will look into it, thanks for the nudge! Some refactoring in the editor wouldn't be too bad anyway. Despite its modularity, some parts of it have grown a little too organic.

tliecau commented 9 years ago

Hi, i'm writing some test case for Rect class in Duality and I've reached to the problem. If I use round method form MathF or Rect with something.5 (f.g 2.5) it is rounded to 2.0 - mathematically speaking it is a bug. However, I've found here : http://stackoverflow.com/questions/977796/why-does-math-round2-5-return-2-instead-of-3-in-c information about something called banker's rounding (now used in project) and rounding away from zero (which will round 2.5 to 3.0). Can You tell me which one we should expect from Duality if it want to round 2.5f?

AdamsLair commented 9 years ago

Hi @WalliePL,

as MathF relies on System.Math for rounding, the same default behavior should be expected. However, I would suggest not to base unrelated unit tests on details like this and instead find a way to test things independently - like not using exact .5f values with implementation-dependent rounding, but instead using something like .4f where rounding behavior is non-ambiguous. :)

tliecau commented 9 years ago

Thank You for answer, i'll do this as You said. There is one more thing, I don't find any information about setting width and height of rect to value less than 0. Should it be possible to set those parameters less than zero? Everything was ok, till Intersects came for testing :). If width and height should be more than 0 then there is no validation for them.

AdamsLair commented 9 years ago

I don't find any information about setting width and height of rect to value less than 0. Should it be possible to set those parameters less than zero?

Width and Height are allowed to be less than zero, so having no check or enforcement there is fine :)

AdamsLair commented 9 years ago

Bumping this as a reminder for myself to increase test coverage in the process of v2.0 implementation.

ilexp commented 8 years ago

Closing this as part of a general cleanup, because it has grown way to unspecific. Testing shouldn't be an always-open issue, but a set of specific issues, a general best-practice and a tag for certain issues to reproduce them with a test before fixing / implement them.