Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

test(warnings): switch from the unmaintained pytest-freezegun to pytest-freezer removing warnings #639

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

Merged
merged 2 commits into from
Dec 22, 2022

Conversation

noirbizarre
Copy link
Member

@noirbizarre noirbizarre commented Dec 16, 2022

Description

Switch from pytest-freezegun which is not maintained anymore and stuck on a legacy freezegun version raising lots of warnings to pytest-freezer its maintained fork by the official pytest-dev github organization which is maintained and up to date, raises no warnings and provide typing.

Checklist

  • Add test cases to all the changes you introduce
  • Run ./scripts/format and ./scripts/test locally to ensure this change passes linter check and test
  • Test the changes on the local machine manually
  • Update the documentation for the changes (N/A)

Expected behavior

No behavior change expected.
Test should still pass but the 2662 warnings raised should have disappeared.

Steps to Test This Pull Request

Run the test suite

Additional context

Dependency change so the dependencies have been re-locked

@codecov
Copy link

codecov bot commented Dec 16, 2022

Codecov Report

❗ No coverage uploaded for pull request base (v3@489096b). Click here to learn what that means.
Patch has no changes to coverable lines.

Additional details and impacted files
@@          Coverage Diff          @@
##             v3     #639   +/-   ##
=====================================
  Coverage      ?   98.42%           
=====================================
  Files         ?       39           
  Lines         ?     1655           
  Branches      ?        0           
=====================================
  Hits          ?     1629           
  Misses        ?       26           
  Partials      ?        0           
Flag Coverage Δ
unittests 98.42% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@Lee-W Lee-W left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @noirbizarre !

@woile I'm planning on merging it this week. Let me know if you want to take a deeper look. Thanks!

@Lee-W
Copy link
Member

Lee-W commented Dec 19, 2022

@noirbizarre I'm thinking maybe we could send this PR directly to main instead of v3. We could rebase it back to v3 later. What do you think?

@noirbizarre
Copy link
Member Author

Except for the lockfile part, it can be sent to main. But in case of rebase or merge on v3, lockfile should be updated (they will conflict).

@Lee-W
Copy link
Member

Lee-W commented Dec 19, 2022

I think we can do the relock for main and relock again after rebasing to v3. What do you think?

@noirbizarre
Copy link
Member Author

I say that because we don't have a lock yet on main so it will be a 2 step move:

  • merge the pyproject.toml change only on main
  • relock on v3 on rebase/merge
    I don't have the proper permissions to do it but I can just cherry-pick and amend the pyproject.toml part on main and keep this PR for v3, merge will be easier I think

@noirbizarre
Copy link
Member Author

Backported in #643 but I realised that given test fixes are not on master tests are not passing on master

@Lee-W
Copy link
Member

Lee-W commented Dec 21, 2022

I just merge #643 back to main. Once main is updated, you can update your local main and rebase it back to your feature branch and than force push to this branch. Thanks for the prompt PR!

@noirbizarre
Copy link
Member Author

Thanks !
Given I can't rebase v3, I just merged main into this branch so it won't conflict on later merges.

@Lee-W
Copy link
Member

Lee-W commented Dec 21, 2022

indeed. I forget that 😱 let me do so now!

@noirbizarre
Copy link
Member Author

noirbizarre commented Dec 21, 2022

Thanks, updated.

But I think your working copy was not up to date and #638 is missing from the rebased v3 (It should not conflict with this PR so you can cherry-pick it on v3 before or after this PR merge without any issue)

@Lee-W
Copy link
Member

Lee-W commented Dec 21, 2022

yep.... do you still have the branch on your local?

@noirbizarre
Copy link
Member Author

noirbizarre commented Dec 21, 2022

No but I just restored it on github in the PR #638

@Lee-W
Copy link
Member

Lee-W commented Dec 21, 2022

ah yes, we could do that. I just push that back to v3

@noirbizarre
Copy link
Member Author

noirbizarre commented Dec 21, 2022

And this is properly rebased on v3
(I kept the style change in a separate commit: it was changed in the previous commit and I believe black was not up to date)

@Lee-W Lee-W merged commit 61c3024 into commitizen-tools:v3 Dec 22, 2022
@noirbizarre noirbizarre deleted the fix/warnings branch December 22, 2022 06:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants