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

🚚(backend) split users test file to improve readability #598

Merged
merged 6 commits into from
Dec 17, 2024

Conversation

mjeammet
Copy link
Member

@mjeammet mjeammet commented Dec 13, 2024

Purpose

"test_api_users" was a single test file of 900+ lines.
We used gitfilesplit to split it into several shorter files
for readability.

Proposal

Description...

  • used https://pypi.org/project/gitfilesplit/ to split test_api_users into several shorter files
  • added init.py file
  • checked 472 tests before and 472 tests after
  • checked history is untouched :
    image
    image

@mjeammet mjeammet requested a review from qbey December 13, 2024 15:55
@mjeammet mjeammet self-assigned this Dec 13, 2024
@mjeammet
Copy link
Member Author

mjeammet commented Dec 13, 2024

@qbey lint-git is required. Did you go into settings and allow force merge for 573 ?

I guess you did because last people workflow was this one : https://github.com/numerique-gouv/people/actions/runs/12142853242 where lint-git fails

@qbey
Copy link
Collaborator

qbey commented Dec 17, 2024

@qbey lint-git is required. Did you go into settings and allow force merge for 573 ?

I guess you did because last people workflow was this one : https://github.com/numerique-gouv/people/actions/runs/12142853242 where lint-git fails

@mjeammet yes, I think I had to ignore the lint-git and force the merge
It's really cool you managed to keep history with just one commit 👍 well done
If you want to be perfect and fix the lint-git, you can rename every commit... but I think it's not mandatory, and a force is way easier and still clean.

@mjeammet
Copy link
Member Author

mjeammet commented Dec 17, 2024

I've tried to rebase on main and git seems very confused

CONFLIT (renommage/renommage) : src/backend/core/tests/test_api_users.py renommé en src/backend/core/tests/users/test_api_users_retrieve.py dans HEAD et en src/backend/core/tests/users/test_api_users_create.py dans ee1d896 (Split test_api_users.py into users/test_api_users_create.py).
error: impossible d'appliquer ee1d896... Split test_api_users.py into users/test_api_users_create.py

@mjeammet mjeammet force-pushed the mpj/split-test-api-users branch from f0e63bf to 38369a8 Compare December 17, 2024 13:00
@mjeammet mjeammet merged commit 38369a8 into main Dec 17, 2024
19 of 20 checks passed
@mjeammet mjeammet deleted the mpj/split-test-api-users branch December 17, 2024 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants