Feature/delete rec additional test data set

[rachmadaniHaryono]

mostly about non integer input and 'max' keyword input.

related #132

[jarun]

Do I need to do anything here?

[rachmadaniHaryono]

Maybe answer the question from 132,start fix with max keywords and set if the test separation is correct?

[jarun]

Answered. Please check https://travis-ci.org/jarun/Buku/jobs/213773882

I think the API delete_rec() is not even called.

[rachmadaniHaryono]

I think the API delete_rec() is not even called

You are right, I will fix it in next commit

[rachmadaniHaryono]

if is_range == True and high == max, is high equal to 3 or 4?

3

try it once again, and imo it should be 4 instead of 3 e.g.

bdb.delete_rec(is_range=True, low=2, high='max')
# should be equal to
bdb.delete_rec(is_range=True, low=2, high=4)
# as in range(2, 4) -> [2, 3]
#
# but if high=3
bdb.delete_rec(is_range=True, low=2, high=3)
# equal to range(2,3) -> [2]

[jarun]

The query we are using is:

'DELETE from bookmarks where id BETWEEN ? AND ?'

So it will delete both 2 and 3 when you pass 2, 3. It doesn't work exactly like range() in this case.

[jarun]

Travis failed. Can you please explain the test case:

test_bukuDb.py::test_delete_rec_negative[-1-0-max-True] FAILED

What do you pass here? How many records in DB? What is max? What are you expecting as output?

[rachmadaniHaryono]

What do you pass here?

setup = None, index = -1, low = 0, high = 'max', is_range = True

https://travis-ci.org/jarun/Buku/jobs/214497928#L614

it expect the database to be cleared. even when high == 'max'.

What is max?

i thought you expect max as acceptable input on delete_rec? from the last thread

Some example acid test values for range:

0 - max
-0 - max
...

e:

How many records in DB?

still 3, maybe i will add after testing add_rec

[jarun]

Initially when I mentioned max I intended MAX_INT. I understand now we refer to max_id in DB as max. So what is the integer value of max in the test case? Is it 3 (as you have 3 records in the DB)?

If you pass: -1, 0, 3, True
all the 3 records will be deleted and the API will return True.

Are you expecting the same output?

[rachmadaniHaryono]

Initially when I mentioned max I intended MAX_INT. I understand now we refer to max_id in DB as max. So what is the integer value of max in the test case? Is it 3 (as you have 3 records in the DB)?

it is my misunderstanding. i thought string max is valid input for low, high and index. the max on current test of is only string 'max'. i will change it to the value of max record on db on next commit

and yes the actual value of max right now is 3.

If you pass: -1, 0, 3, True
all the 3 records will be deleted and the API will return True.

Are you expecting the same output?

for this

setup = None, index = -1, low = 0, high = 'max', is_range = True

yes, just as you said.

all the 3 records will be deleted and the API will return True.

[jarun]

So I guess you have to change max to len(records_in_DB) in the test cases, right? I think the ones with max are failing.

[jarun]

I believe the following test cases are sufficient to test delete_rec with non-int:

test_bukuDb.py::test_delete_rec_on_non_interger[a-a-1-True] PASSED
test_bukuDb.py::test_delete_rec_on_non_interger[a-a-1-False] PASSED
test_bukuDb.py::test_delete_rec_on_non_interger[a-1-a-True] PASSED

The other test cases redundantly test the same paths and increase the time required to complete the tests.

I think we should resort to intelligent manually built up test cases than going for matrix based brute-force test.

For another example, I see tons of test_print_single_rec. Why? I wouldn't even bother testing this API with so many test cases.

[rachmadaniHaryono]

I think we should resort to intelligent manually built up test than going for matrix baed brute-force test.

i have the same problem too. so today i actually look up for python-hypothesis and use it on test.

with this i found a bug regarding big integer with 2 situations

• when using it with index, it will raise OverflowError. max value of integer that sqlite3 3 can handle is 9223372036854775807 http://stackoverflow.com/questions/4448284/sqlite3-integer-max-value
• when using it with range (and normalized low value == 0), it will take a long time. it is caused by this loop for index in range(low, high + 1):. i think this should be fixed.

In another example, I see tons of test_print_single_rec. Why?

i will change the test with hypothesis and see if there is another bug.

other things is about test package. the current test packages are put on two place, travis and setup.py. this should be only on setup.py and travis can install with python setup.py develop.

[jarun]

this should be only on setup.py and travis can install with python setup.py develop.

Sure!

[rachmadaniHaryono]

Sure!

ok, i will do it. please fix the loop for is_range==True.

e: i am wrong with python setup.py develop. it only used so installed package change everytime the current package change.

to merge these travis and setup, i have to add extras in setup.py. developer can install it with pip install -e .[tests] (e flag is optional, but recommended).

relate so http://stackoverflow.com/a/18879288/1766261

it is working on travis, but not on zsh(i will fiind out later)

e2: for zsh the command is pip install -e .\[extra\] link: http://stackoverflow.com/a/30239714/1766261 (see the comment)

[jarun]

please fix the loop for is_range==True

What's the issue with that?

[jarun]

Why are you using int(MAX_SQLITE_INT + 1), specifically why are you adding 1?

[rachmadaniHaryono]

Why are you using int(MAX_SQLITE_INT + 1), specifically why are you adding 1?

i am trying to recreate the same OverflowError issue with index. but it can't be tested because this part of the code

for index in range(low, high + 1):
    self.compactdb(index, delay_commit=True)

when high value is unnecessarily high enough this will take time and and cause the error on travis. travis will raise error when a test case don't respond after ~10m

here is similar sqlite issue which happen to django https://code.djangoproject.com/ticket/27397

---

the actual number of MAX_SQLITE_INT is 2^63-1 instead of 2^47. it will be fixed on next commit

[jarun]

Thanks for the investigation! Let's pass MAX_SQLITE_INT. I can add a check before range but I believe we may never hit this issue in reality.

[jarun]

Also, please reduce the number of tests for test_delete_rec_on_non_integer.

I'm even thinking of removing the non-int case because we are clear we want integers. Feel free to take a call on this.

[rachmadaniHaryono]

i did several skip, but unfortunately skip + parametrize isnt working on py3.3 and py3.4, possibly related to this pytest-dev/pytest#1296

i think this is finished, right?

next would be

• simplify test_print_single_rec
• test print_rec
• tox

e:

I can add a check before range but I believe we may never hit this issue in reality.

imo it is better to fix this.

even if the number will not met in reality, any high number input will make program take unnecessary time to process

[jarun]

i think this is finished, right?

As I mentioned earlier, we should reduce the test cases for delete_rec with non-int to just 3 cases:

test_bukuDb.py::test_delete_rec_on_non_interger[a-a-1-True] PASSED
test_bukuDb.py::test_delete_rec_on_non_interger[a-a-1-False] PASSED
test_bukuDb.py::test_delete_rec_on_non_interger[a-1-a-True] PASSED

rest of the cases are redundant.

[jarun]

Thank you!

[jarun]

next would be

simplify test_print_single_rec
test print_rec
tox

yes, please proceed.