Skip to content
This repository has been archived by the owner on Jan 21, 2024. It is now read-only.

add basic get service tests #18

Merged
merged 7 commits into from
Jan 16, 2021
Merged

Conversation

boleaczek
Copy link
Contributor

Hi,
I have written some tests for get service. Right now only cases where request fails.

For OK case where file is found and authorization is successful I think we need to create an user(this is risky because db could be polluted with such test users if test panics) or mock db related stuff(this will require some refactoring). I'm new to actix, so maybe you'll have some better ideas.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@AaronErhardt
Copy link
Member

Thanks a lot for contributing!

The code looks pretty solid, I just think it makes sense to put it into src/tests/files/get.rs so it's clear it belongs to the file services.

I've quickly added a util module with utility methods that are useful for testing. You should be able to use test_user() to obtain a new user with valid JWT that will be automatically removed from the database when the variable is dropped. The code it pretty ugly but it should work. In general a lot refactoring is planned but for now I don't have time for that.

@boleaczek
Copy link
Contributor Author

Thanks. I have changed test_user so that it returns user id(we need it for test file creation) and uses app state provided by caller to keep secret the same between them.

If I will be able to, I'll gladly help with refactoring.

@AaronErhardt
Copy link
Member

Thanks a lot for investing your time into this project :)

The refactoring will probably begin in a few weeks when the other contributors and I have more time (some of us are busy with exams). Yet if you want you can already start improving or adding some code. There are several problems like duplicated code and better modularization I'd like to fix in the code base. Issue #17 sums up most of the planned efforts in this direction but we are open for new ideas as well. Also there's a new Matrix server where we discuss our plans, if you want you can join us there (I hope the link in #16 is still valid).

@AaronErhardt AaronErhardt merged commit 63e6d91 into Trioxidation:master Jan 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants