-
-
Notifications
You must be signed in to change notification settings - Fork 301
fix(commit): correct the stage checker before commiting #420
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
Conversation
Codecov Report
@@ Coverage Diff @@ ## master #420 +/- ## ========================================== - Coverage 97.90% 97.90% -0.01% ========================================== Files 39 39 Lines 1383 1382 -1 ========================================== - Hits 1354 1353 -1 Misses 29 29
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Hi @xaviml thanks for the contribution!
I do not understand the issue, so I'd like to ask you a test where it would fail before this PR and fixed after. It would also work as a way to document the behavior, and by seen it, I think I would understand better 👍🏻
Sorry for the inconvinience 😅
Hi @woile ,
Of course, I totally understand. Is there any test example in the test suite that actually test with the real git command? Is it okay if I do so? Otherwise, I don't see a way to test this properly since it only calls the git command.
Regards,
Xavi M.
You can write it in https://github.com/commitizen-tools/commitizen/blob/master/tests/test_git.py
You can call git with this:
Line 148 in 79b3156
By using this fixture you'll have a git folder ready:
Line 60 in 79b3156
Example:
@pytest.mark.usefixtures("tmp_commitizen_project") def test_git_something(): c = cmd.run("git something")
f720d2b to
a2d6312
Compare
Hi @woile,
Thank you for pointing me to the right test file. I did the following:
- Rename existing is_staging_clean test to
test_is_staging_clean_when_adding_file - Add
test_is_staging_clean_when_updating_file
The new test fails in the second assert with the old code. The new code now checks correctly if stage is clean or not.
The bug was (as described in #418) that the is_stage_clean worked when adding a file (as it was tested), but not when modifying an existing file (it was not tested).
Let me know if anything needs changing.
Regards,
Xavi M.
I see it now, wow I thought this was working.
Thanks!
Uh oh!
There was an error while loading. Please reload this page.
fixes #418
Checklist
./script/formatand./script/testlocally to ensure this change passes linter check and testThis part of the code was not originally tested, since changing the code did not break the existing tests. If interested, I could add tests for this with real git calls.
I did not add this to the documentation or README since it fixes a behaviour that was supposed to happen already.