Skip to content

Role.role_name can be null #7688

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.

Sign up for GitHub

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

Jump to bottom
Closed
di opened this issue Mar 27, 2020 · 1 comment · Fixed by #7697
Closed

Role.role_name can be null #7688

di opened this issue Mar 27, 2020 · 1 comment · Fixed by #7697
Labels
developer experience Anything that improves the experience for Warehouse devs testing Test infrastructure and individual tests

Comments

@di
Copy link
Member

di commented Mar 27, 2020

Pairing with @ewdurbin on #2745 we noticed that Role.role_name can be null:

https://github.com/pypa/warehouse/blob/32f621a359bfa7ecb84d8b0ff77e675aae2cd9e1/warehouse/packaging/models.py#L64

Enforcing this to be nullable=False causes a lot of our tests to fail due to our ProjectFactory and UserFactory implicitly creating roles without names, since they bypass the RoleFactory.

We should update our test fixtures to allow us to set nullable=False.

Some possible leads: https://factoryboy.readthedocs.io/en/latest/reference.html
@di di added testing Test infrastructure and individual tests developer experience Anything that improves the experience for Warehouse devs labels Mar 27, 2020
@ewdurbin
Copy link
Member

After #7689, it looks like only 4 tests are left depending on the implicit Role creation here.

I'm not sure if supporting

test_project = ProjectFactory(users=[test_user0, test_user1])
or
test_user = UserFactory(projects=[test_project0, test_project1])

is worth it really.

ewdurbin added a commit that referenced this issue Mar 28, 2020
@ewdurbin
Roles: db migration to make role_name not nullable
af983e9 
Depends on #7689, fixes #7688
ewdurbin added a commit that referenced this issue Mar 30, 2020
@ewdurbin
Roles: db migration to make role_name not nullable
80e676a 
Depends on #7689, fixes #7688
@ewdurbin ewdurbin mentioned this issue Mar 30, 2020
Merged
ewdurbin added a commit that referenced this issue Mar 30, 2020
@ewdurbin
Roles: db migration to make role_name not nullable (#7697)
345661f 
* Roles: db migration to make role_name not nullable

Depends on #7689, fixes #7688

* lint
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
developer experience Anything that improves the experience for Warehouse devs testing Test infrastructure and individual tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Roles: db migration to make role_name not nullable pypi/warehouse
2 participants
@di @ewdurbin