full-stack-fastapi-template

full-stack-fastapi-template copied to clipboard

See discussion here: https://github.com/tiangolo/full-stack-fastapi-template/discussions/1135#discussioncomment-9508587

This was changed in the following PR: https://github.com/tiangolo/full-stack-fastapi-template/pull/632

Changing it back to settings.FIRST_SUPERUSER_PASSWORD makes it work again.

I think the issue is that after this test, the password is changed to something else and the other tests can't authorize.

There was another fix in that discussion here: https://github.com/tiangolo/full-stack-fastapi-template/discussions/1135#discussioncomment-8938134

Don't know what is preferred, let me know.

Forgot to add...

To reproduce:

1. Clone master
2. Edit FIRST_SUPERUSER_PASSWORD in .env to something other than "changethis"
3. docker compose up
4. Exec into backend container
5. run bash ./scripts/test.sh

Hi @alejsdev. Before I merge, there was another comment here on this: https://github.com/tiangolo/full-stack-fastapi-template/discussions/1135#discussioncomment-10078421

Should we consider this? I think the API functionality is still tested, but the commenter is right that the actual password change isn't really tested...

I think one solution is caching original password, changing it, testing the change, and then resetting it back to the original.

I see, I think it would be better to create a new user and use it to reset his password or using @swelborn approach because changing the password of the first superuser would break the other tests, and using the same password doesn't make much more sense, I think it would be better to refactor this test cc @tiangolo @estebanx64 :thinking:

Hey @javadzarezadeh and @alejsdev - I see that you both approved this. Could merge for now to avoid tests failing for new users, and then refactor a different way with a new issue?

Hi @swelborn, thanks for your patience, this fix was handled in #1499 by refactoring the whole test. I'll close this one but thanks for your effort. :raised_hands: