jabacat / jml

JABACAT-created machine learning library from scratch.
5 stars 5 forks source link

Add unit testing framework and tests for existing code (and assorted fixes and breaking API changes) #23

Closed Sophon96 closed 11 months ago

Sophon96 commented 11 months ago

Using catch2 and integration with meson's testing harness (so tests can be run with meson test).

Sophon96 commented 11 months ago

Incomplete, blocked for test requirements from @adamhutchings for the matrix and vector classes.

adamhutchings commented 11 months ago

Are we merging this after #25?

Sophon96 commented 11 months ago

Are we merging this after #25?

No, I need to write tests for the existing code as well (though this is blocked by #25 as well).

adamhutchings commented 11 months ago

Is development on this able to proceed or are we still waiting on something else?

Sophon96 commented 11 months ago

Is development on this able to proceed or are we still waiting on something else?

Waiting on nothing, just time and writing tests. I think it would be interesting if we want to do test-driven development or behavior-driven development for v1 (i.e. draft the API, write all the tests, and once all tests pass v1 is complete).

adamhutchings commented 11 months ago

I think that is a worthwhile discussion to have, but if we have the basis of testing set up, we can probably close this PR and add more tests as they come (or however else we agree to do it).

Sophon96 commented 11 months ago

Blocked #38

Sophon96 commented 11 months ago

@adamhutchings @JakeRoggenbuck Can any of y'all find where I messed up the code in the test? tests/core/test_matrix.cpp

There are numerous bugs I've found in the actual lib source code. I'm working on fixing them right now.

Sophon96 commented 11 months ago

(this should be squashed as well, this git history is unimaginably bad)

Sophon96 commented 11 months ago

TODO before this can be merged

Sophon96 commented 11 months ago

C API is not tested.

Sophon96 commented 11 months ago

oh wait i need to fix ci again don't merge

Sophon96 commented 11 months ago

@adamhutchings @JakeRoggenbuck Ready to review