yellowstonegames / SquidLib

Useful tools for roguelike, role-playing, strategy, and other grid-based games in Java. Feedback is welcome!
Other
448 stars 46 forks source link

Add unit tests for squidpony.squidmath.NumberTools #205

Closed sudo-jaa closed 5 years ago

sudo-jaa commented 5 years ago

Hi,

I've analysed your code base and noticed some gaps in the coverage of squidmath/NumberTools.java. I've written some tests for the functions in NumberTools with the help of Diffblue Cover. Hopefully these tests should help you detect any regressions caused by future code changes. If you would find it useful to have additional tests written for SquidLib, I would be more than happy to look at a particular class and you help achieve a higher coverage figure.

tommyettinger commented 5 years ago

Very nice, I appreciate that you took the time to review existing tests here, such as they are (my existing tests aren't currently very good). You might get better feedback in the future by linking to discussions under Diffblue-benchmarks (like so) so people have confidence that this isn't a pull-request-shotgun approach done by automated methods (that was an issue in the past with a "cleanup PR" that had bad semantic changes, outside the tests and throughout the code).

NumberTools is a particularly odd class because it has multiple potential points of failure -- changes in the regular NumberTools.java file need to be echoed in the Google Web Toolkit super-sourced version of the class in the emu folder of squidlib-util. Super-sourcing allows the GWT version to use JavaScript's typed arrays when compiling Java to JavaScript, which greatly improves some methods' performance relative to GWT's default equivalents. But if NumberTools adds a method in the normal Java file and not in the super-sourced emu file, compilation will fail on GWT. It would be nice if there was some simple way of checking that all method signatures in the regular NumberTools are also present in the emu path, even if there's no way of running them without GWT. GWT is barely used these days it seems, so this probably isn't the best suggestion for a new feature, but if DiffBlue can already check if a class duplicates another class' API, which it should in the case of super-sourcing, that would be a good feature to know about.