louisabraham / pydivsufsort

Python package for string algorithms ➰
MIT License
38 stars 4 forks source link

Add Optional Unit Test For Large Inputs #7

Closed seanlaw closed 4 years ago

seanlaw commented 4 years ago

@louisabraham Would it be helpful to start writing unit tests or would it be premature/overkill? I could possibly help with this if you want to provide some guidance on what needs to be/should be tested. I typically use pytest

louisabraham commented 4 years ago

Look at the README :)

louisabraham commented 4 years ago

What worries me is the need to test for inputs larger than 2**31 (thus needing divsufsort64). This is not something anybody can do so I don't want to include it in the tests.

seanlaw commented 4 years ago

Look at the README :)

Ha! Glad we're on the same page. Just let me know how I can help

louisabraham commented 4 years ago

Could you write a test for huge inputs and execute it? There is probably a way to specify some tests as optional in pytest.

seanlaw commented 4 years ago

When you say 2**31, are you saying:

inp = np.random.randint(2**31, dtype=np.uint8)

Is the goal to actually verify the output or only to ensure that the input can be handled (and assume that the output is correct)?

To handle the long/slow running tests, we can skip them from the command line according to this documentation

louisabraham commented 4 years ago

That's precisely the issue: in theory, I would have to check the result. However, I don't think we should lose too much time on that. There is a function in libdivsufsort to check the result, I'll implement it later (#11) so we don't really to check correctness.

louisabraham commented 4 years ago

That's probably overkill. The original code is the same for the 32 and 64 bits versions, with just some flags changing, so I'm comfortable with testing only the 32 bits code.