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: make use of the stdlib unittest.mock for python 3.3+ #346

Closed

Conversation

eli-schwartz
Copy link
Contributor

Fixes: #339

I don't really like the idea of a "compat" module but if one must...

@andreafrancia
Copy link
Owner

Test does not pass.

@eli-schwartz
Copy link
Contributor Author

@andreafrancia please reopen this PR so that I can make the trivial typo fix for the tests.

@eli-schwartz
Copy link
Contributor Author

eli-schwartz commented Jul 8, 2024

failed 5 minutes ago in 13s

Does not give me a whole lot of time to look at CI results and try to fix them... github does not run CI automatically so I did not have a chance to see this issue until right now.

@andreafrancia
Copy link
Owner

You could run pip install on a clean python 2.7 virtual environment.
To install Python 2.7 you can use pyenv.
To create a virtual environment you can use virtualenv

I prefer not to keep open the PR because I can't know how much time will pass before a contributor make the following push.
But I think you can keep pushing code.

In any case you can fork and activate Actions on your repository, I don't need any of my approval to craft a commit that passes all the tests on this trash-cli repository.

@eli-schwartz
Copy link
Contributor Author

I prefer not to keep open the PR because I can't know how much time will pass before a contributor make the following push.
But I think you can keep pushing code.

If you push to a closed PR then GitHub does not let you re-open it until you restore the exact commit sha1 that was the HEAD commit for the PR at the time the PR was closed. It's a huge hassle and requires reverting any pushes anyway.

For faster turnaround, it's advisable to override GitHub's default setting -- which doesn't run CI for any first time contributor -- to instead just skip running for "new GitHub users". The purpose of this setting is actually because once upon a time GitHub simply ran CI all the time, but people started spamming GitHub by creating dummy accounts and creating PRs to add Bitcoin miners. There is no implied or explicit problem with automatically running CI by first time contributors to a project that are also longstanding GitHub users (I've had a GitHub account for a decade and made thousands of contributions to one project or another per year, so GitHub knows very well I'm not a spam account auto created by a Bitcoin bot). Unfortunately, GitHub just has horribly bad default settings. :(

Anyway, I am definitely available today to make additional pushes, no worries.

@andreafrancia
Copy link
Owner

I'm not changing the default settings. Please do test your code before sending a pull request.

@eli-schwartz
Copy link
Contributor Author

I'm not changing the default settings.

You're shooting yourself in the foot, but fine.

Please do test your code before sending a pull request.

I did. The code passed all tests.

The github actions integration didn't.

@eli-schwartz
Copy link
Contributor Author

I prefer not to keep open the PR because I can't know how much time will pass before a contributor make the following push.

I got tired of waiting. #347

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.

Don't force user to install old mock library
2 participants