Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tests fail after the first run #109

Closed
laurentS opened this issue Feb 7, 2020 · 5 comments
Closed

Tests fail after the first run #109

laurentS opened this issue Feb 7, 2020 · 5 comments
Labels

Comments

@laurentS
Copy link

laurentS commented Feb 7, 2020

I got very confused by the tests that were always passing in CI, but failing in my local dev environment.
After trying a variety of ideas, it turns out that the first test run after pruning all docker images and containers passes, but subsequent ones fail with:

self = <app.crud.crud_user.CRUDUser object at 0x7f6dddde08d0>, db_session = <sqlalchemy.orm.scoping.scoped_session object at 0x7f6dddac4f10>

    def update(
        self, db_session: Session, *, db_obj: ModelType, obj_in: UpdateSchemaType
    ) -> ModelType:
        obj_data = jsonable_encoder(db_obj)
>       update_data = obj_in.dict(skip_defaults=True)
E       AttributeError: 'User' object has no attribute 'dict'

app/app/crud/base.py:44: AttributeError

in two tests: test_get_users_normal_user_me and test_create_user_by_normal_user.

To reproduce, simply run the tests twice in a row from a fresh install. The first run will pass, the second one will fail.
To "reset" the tests, docker-compose exec db psql -U postgres and \c app then delete from public.user where id>1; and the tests will pass again.

I suspect this might have to do with some dependency that changed its api at some point, and the change didn't get caught in CI because this code path is not actually tested.

@StephenBrown2
Copy link
Contributor

Yeah, that does seem like a problem. 100% line coverage isn't incredibly important, but most code paths should probably be. Here's a coverage report I just ran on my pydantic_basesettings branch:

----------- coverage: platform linux, python 3.7.6-final-0 -----------
Name                                Stmts   Miss  Cover   Missing
-----------------------------------------------------------------
app/api/api_v1/api.py                   7      7     0%   1-9
app/api/api_v1/endpoints/items.py      45     45     0%   1-104
app/api/api_v1/endpoints/login.py      51     51     0%   1-96
app/api/api_v1/endpoints/users.py      66     66     0%   1-156
app/api/api_v1/endpoints/utils.py      17     17     0%   1-33
app/api/utils/db.py                     3      3     0%   1-5
app/api/utils/security.py              31     31     0%   1-45
app/backend_pre_start.py               19     19     0%   1-36
app/celeryworker_pre_start.py          19     19     0%   1-36
app/core/celery_app.py                  3      3     0%   1-5
app/core/config.py                     53      4    92%   26, 36, 47, 68
app/core/jwt.py                        13     13     0%   1-19
app/crud/base.py                       37      7    81%   30, 33-38
app/crud/crud_item.py                  17      1    94%   25
app/crud/crud_user.py                  27      1    96%   34
app/db/base.py                          3      3     0%   3-5
app/db/init_db.py                       9      9     0%   1-24
app/initial_data.py                    13     13     0%   1-21
app/main.py                            16     16     0%   1-29
app/schemas/msg.py                      3      3     0%   1-5
app/schemas/token.py                    6      6     0%   1-10
app/utils.py                           60     60     0%   1-111
app/worker.py                           7      7     0%   1-11
-----------------------------------------------------------------
TOTAL                                 932    428    54%

30 files skipped due to complete coverage.

There's definitely room for improvement, especially with those 0% covered files...

@michaeloliverx
Copy link

I noticed this yesterday, I thought it was my own fault as I had added some features and was searching for bugs. I happened to run docker-compose down and then up and they all passed again.

@laurentS
Copy link
Author

laurentS commented Feb 7, 2020

Looking a bit further into it, the issue should be fixed by #107 and this change needed in app/tests/utils/user.py:

@@ -36,8 +36,8 @@ def authentication_token_from_email(email):
     if not user:
         user_in = UserCreate(username=email, email=email, password=password)
         user = crud.user.create(db_session=db_session, obj_in=user_in)
     else:
         user_in = UserUpdate(password=password)
-        user = crud.user.update(db_session, obj_in=user, db_obj=user_in)
+        user = crud.user.update(db_session, obj_in=user_in, db_obj=user)
 
     return user_authentication_headers(get_server_api(), email, password)

@tiangolo
Copy link
Member

Hey there! It seems this was solved in #106 , right?

Sorry for the long delay! 🙈 I wanted to personally address each issue and they piled up through time, but now I'm checking each one in order.

@github-actions
Copy link

github-actions bot commented Nov 2, 2022

Assuming the original issue was solved, it will be automatically closed now. But feel free to add more comments or create new issues.

@github-actions github-actions bot closed this as completed Nov 2, 2022
alejsdev pushed a commit that referenced this issue Dec 19, 2024
Bumps [pulumi/actions](https://github.com/pulumi/actions) from 3 to 5.
- [Release notes](https://github.com/pulumi/actions/releases)
- [Changelog](https://github.com/pulumi/actions/blob/main/CHANGELOG.md)
- [Commits](pulumi/actions@v3...v5)

---
updated-dependencies:
- dependency-name: pulumi/actions
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants