⚡️ Improve backend test performance by patching password hashing

[saltie2193]

Improve performance of backend tests by patching password hashing during tests.

NOTE: To reduce interference with preexisting data, all existing users and items are now deleted before the tests are run. This should not be noticable outside of testing, since all users and items already have been deleted after testing anyway.

[mdrxy]

This substantially reduces testing time - thank you!

Only thing I noticed is that session.execute should become session.exec due to deprecation.

[saltie2193]

At least according to mypy, session.exec of a SQLModel Session does not have an overload that would allow for passing a delete statement. Using it will fail the linting. However it seems to work anyway.

mypy output when using session.exec:

app/tests/conftest.py:28: error: No overload variant of "exec" of "Session" matches argument type "Delete"  [call-overload]
app/tests/conftest.py:28: note: Possible overload variants:
app/tests/conftest.py:28: note:     def [_TSelectParam] exec(self, statement: Select[_TSelectParam], *, params: Mapping[str, Any] | Sequence[Mapping[str, Any]] | None = ..., execution_options: Mapping[str, Any] = ..., bind_arguments: dict[str, Any] | None = ..., _parent_execute_state: Any | None = ..., _add_event: Any | None = ...) -> TupleResult[_TSelectParam]
app/tests/conftest.py:28: note:     def [_TSelectParam] exec(self, statement: SelectOfScalar[_TSelectParam], *, params: Mapping[str, Any] | Sequence[Mapping[str, Any]] | None = ..., execution_options: Mapping[str, Any] = ..., bind_arguments: dict[str, Any] | None = ..., _parent_execute_state: Any | None = ..., _add_event: Any | None = ...) -> ScalarResult[_TSelectParam]
app/tests/conftest.py:29: error: No overload variant of "exec" of "Session" matches argument type "Delete"  [call-overload]
app/tests/conftest.py:29: note: Possible overload variants:
app/tests/conftest.py:29: note:     def [_TSelectParam] exec(self, statement: Select[_TSelectParam], *, params: Mapping[str, Any] | Sequence[Mapping[str, Any]] | None = ..., execution_options: Mapping[str, Any] = ..., bind_arguments: dict[str, Any] | None = ..., _parent_execute_state: Any | None = ..., _add_event: Any | None = ...) -> TupleResult[_TSelectParam]
app/tests/conftest.py:29: note:     def [_TSelectParam] exec(self, statement: SelectOfScalar[_TSelectParam], *, params: Mapping[str, Any] | Sequence[Mapping[str, Any]] | None = ..., execution_options: Mapping[str, Any] = ..., bind_arguments: dict[str, Any] | None = ..., _parent_execute_state: Any | None = ..., _add_event: Any | None = ...) -> ScalarResult[_TSelectParam]
Found 2 errors in 1 file (checked 38 source files)

I'm not sure what's the better option here. Do we ignore the type checker and provided signature for session.exec? Or, do we insist on not using deprecated methods were it seems like there is no replacement?

[YuriiMotov]

Sorry for pushing to your branch, it was simpler than explaining this in review.. If you don't like changes, feel free to revert commits or use only part of suggested changes.

I suggest simplify tests:

• Since we only patch one module, we don't need that complex things with ExitStack.
• Added if..else condition in disable_password_hashing to make logic clearer
• Explicitly added disable_password_hashing dependency to db fixture
• Removed init_db_fixture (moved init logic to db fixture)
• Simplified docstrings and comments (IMO, some of them were too verbose, some were incorrect)
• Removed unnecessary second test from test_login_with_hashing.py

[github-actions[bot]]

This pull request has a merge conflict that needs to be resolved.

[saltie2193]

That's basically the original proposal now so looks fine to me.

I hope it's fine I reverted the changes to keep the patch_passwors_hashing utility. I think this way it's a bit cleaner and it can be reused easily. If you feel strong about not having it feel free to remove it again.