Add some tests

[qwerty287]

Try to increase test coverage by adding some (partially useless) tests.

[codecov]

Codecov Report

Merging #1395 (2876dcd) into master (89fcba4) will increase coverage by 0.71%.
The diff coverage is n/a.

[ildyria]

I would wait for #1403 as it changes Legacy quite a lot and some of those code may already have changed.

[nagmat84]

I had a look at the changes since my last review. Everything fine so far except the one open comment which can only be closed after #1351. Except from that I am fine with the PR.

However, it would be possible to even improve on this PR further (not I "must", only something which crossed my mind). The tests for the search feature only check whether the uploaded photo is found. There are no negative tests, i.e. that an uploaded photo which does not match the search criteria is not found. This means a silly implementation of the search feature which always simply returns the whole DB without actually searching anything would also pass the tests. Hence, what I would have done is to upload two photos in each of the tests, e.g. the Night photo and the Mongolia photo, search and check that the search result only contains one element and the correct element. But I also understand that this might be out of the scope of this PR. I am already happy that we have more tests.

[nagmat84]

However, it would be possible to even improve on this PR further (not I "must", only something which crossed my mind). The tests for the search feature only check whether the uploaded photo is found. There are no negative tests [...]

Added negative tests myself in e5d042a.

[nagmat84]

I would wait for #1403 as it changes Legacy quite a lot and some of those code may already have changed.

@ildyria I assume you are speaking about the changes introduces by this PR in f732f5d?

I am not sure why @qwerty287 made this part of this PR. To me these changes seem unrelated to the purpose of this PR. I assume if f732f5d was reverted, then the only changes introduced by this PR are more tests and the PR is fine for merge, right?

[ildyria]

@ildyria I assume you are speaking about the changes introduces by this PR in f732f5d?

Yes.

[nagmat84]

@qwerty287 Do you mind to revert f732f5d? I am not sure if this should be part of this PR at all and it will be mostly outdated after #1403. Without this commit, we could merge this PR right away.

[qwerty287]

This is not related to the tests. But I noticed that these methods will always return the same result. The removed methods noLogin and SetPassword return always false (due to the version check). Thus I removed them and their usages. I can revert it in a few days.

[qwerty287]

@nagmat84 @ildyria Reverted.