-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Bump pytest to version 7 #2863
Bump pytest to version 7 #2863
Conversation
The plugin |
The Should I mine a new block by the |
I'd rather address it in the def test_eth_get_balance_with_block_identifier(self, w3: "Web3") -> None:
# send some money to the coinbase account to ensure later balance > genesis
w3.eth.send_transaction({"to": w3.eth.coinbase, "value": 1, "gas": 21000})
super().test_eth_get_balance_with_block_identifier(w3) thoughts? |
Thanks for @fselmo! Your suggestion is great and clear. I push the same commit here. But I have a dumb question, why did you choose to send some value to the coinbase address? If any transaction is sent (same as block mined in |
Frankly, I don't remember off the top of my head if the |
I think it does. The test works until the execution order changes. Since it is to test get_balance with a block number, I think sending a transaction to the coinbase address is a good idea, no matter where the block reward goes. I was curious about the block reward. 🙂 |
Yeah it should handle all of that appropriately but the value transfer is more explicit 👍🏼 |
8962916
to
c60535d
Compare
I saw the PR #2887 dealing with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution @e3243eric! I left a few comments that I can get to in the next few days if you aren't able/willing.
newsfragments/2863.bugfix.rst
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is a dev dependency change, let's call this filename newsfragments/2863.internal.rst
setup.py
Outdated
@@ -28,10 +28,9 @@ | |||
"flaky>=3.7.0", | |||
"hypothesis>=3.31.2", | |||
"importlib-metadata<5.0;python_version<'3.8'", | |||
"pytest>=6.2.5", | |||
"pytest==7.2.2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If possible, I'd rather bump pytest>=7.0.0
to allow a wider range. Does that work here?
Thanks for all the reviews. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @e3243eric! Looks good to me!
What was wrong?
Related to Issue #2859. From CI warning:
DeprecationWarning: You're using an outdated version of pytest. Newer releases of pytest-asyncio will not be compatible with this pytest version. Please update pytest to version 7 or later.
How was it fixed?
Bump
pytest
to version 7.Todo:
Cute Animal Picture