rogueserver icon indicating copy to clipboard operation
rogueserver copied to clipboard

Write tests for account

Open slsyy opened this issue 1 year ago • 3 comments

please tell me, if you like the approach

tests need a little bit more cleaning and polishing, but i post it early to gather a feedback

i can also test other routes in the future

slsyy avatar May 11 '24 15:05 slsyy

Looking great here, however, instead of validating the current (wrong) behavior, please change the tests to validate the desired behavior as indicated, so that the tests fail and can be fixed. Note: Do not fix them in this PR please, as that may require those changes to be mirrored to the frontend.

My approach was to not change anything due to the those changes to be mirrored to the frontend except obvious bugs like a fix in a change password handler, which does not work right now

So next steps: I will fix the API first with backend <-> frontend manual tests, then I want to merge that one with correct behavior already on master

slsyy avatar May 11 '24 23:05 slsyy

My approach was to not change anything due to the those changes to be mirrored to the frontend except obvious bugs like a fix in a change password handler, which does not work right now

So next steps: I will fix the API first with backend <-> frontend manual tests, then I want to merge that one with correct behavior already on master

Having failing tests is fine, I would merge the PR regardless.

UpcraftLP avatar May 11 '24 23:05 UpcraftLP

Having failing tests is fine, I would merge the PR regardless.

If it is not blocking anything (like docker build and push, i don't know) then why not

slsyy avatar May 11 '24 23:05 slsyy