-
Notifications
You must be signed in to change notification settings - Fork 108
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New tests are required #173
Comments
@vkostyukov, hello! |
Hi @ilya-murzinov, that's correct! New tests are something that is needed right now. I'm in the middle to introduce new operations, so we have to be able to test them. I would suggest you to start with infrastructure. There are already tests in This is not an easy issue but I believe it's possible. Please feel free to ask any questions in this issue and submit your PR either in |
Hi Ilya @ilya-murzinov, Plsee see the issue #174 for more details. I think it would be a good idea handle these issues together in order to not do the same work twice. |
@vkostyukov, ok |
Sure! I'm not a big expert in writing tests that's for sure. So, we can turn it in any direction we want. That is a good time for changes in a whole |
@vkostyukov, I have a good experience in software testing (I'm actually QA automation specialist right now), but I can't say that it's gonna be easy for me. I can only promise to do my best :) If we want to start from scratch, it's a good idea to create a branch for it and just remove test folder there completely. Then we can start. |
Right. I'm ok with rewriting everyting. The only thing I care is all the tests that are already in la4j. Some of them are good regression-tests. We somehow have to migrate tests to the new infrastructure rather then inventing new test-cases. |
@vkostyukov, yes, of course, I am going to base new tests on old ones. |
@ilya-murzinov that's a nice plan. Tests are primitive. Everything is bult around factoires (that can produce vectors/matrices). There are 4 factories: The current implementation is based on a simple idea having an abstract test classes (i.e., |
@vkostyukov I'm very sorry, but I don't think I'm capable of solving this issue now. I'm experiencing lack of motivation for such a big thing. May be I should try to do something smaller for the start. |
Hi @ilya-murzinov, No worries! How about take a look at the issue #172. Just refactor the methods to use a proper agument times. Also, don't forget to mark the old methods depricated. |
According to #216, new |
In order to test new operations new tests should be introduced. We have to test all the possible combinations of pair: Sparse-Dense. New tests should also test inPlace operations.
The text was updated successfully, but these errors were encountered: