async more filter tests #2732
async more filter tests #2732
Conversation
pacrob
force-pushed
the
further-async-filter-testing
branch
2 times, most recently
from
02c7e9e to
36e543a
Compare
pacrob
force-pushed
the
further-async-filter-testing
branch
from
36e543a to
959674e
Compare
tests/core/filtering/conftest.py
Outdated
| @pytest.fixture(scope="module") | ||
| def async_create_filter(request): | ||
| return async_partial(return_filter) | ||
| async def async_return_filter(contract=None, args=[]): |
There was a problem hiding this comment.
I don't think it makes sense to have default values for contract and args here since we don't account for those values in the function. For example, if args = [], line 200 below will raise an IndexError. What do you think about removing the default values?
| async def async_return_filter(contract=None, args=[]): | |
| async def async_return_filter(contract, args): |
or even pass in event_name and _kwargs directly:
| async def async_return_filter(contract=None, args=[]): | |
| async def async_return_filter(contract, event_name, _kwargs): |
There was a problem hiding this comment.
+1 for @kclowes' comments here
There was a problem hiding this comment.
I went with first option, changed in the sync version I copied it from too.
|
|
||
| @pytest.fixture(scope="module") | ||
| def emitter(w3, Emitter, wait_for_transaction, wait_for_block, address_conversion_func): | ||
| return _emitter_fixture_logic( |
There was a problem hiding this comment.
nit: I'm not sure I see the value in having a function just pass on it's args to another function. Can we move the logic back here and avoid some indirection? Alternatively, if this is used in more places than it looks like it is in this PR, can we pull the fixture up a few levels?
There was a problem hiding this comment.
The other use is in the local conftest.py, which came through in the previous PR. The fixture is redefined here due to scope. I had pulled the logic of the longer fixtures out to utils.py to avoid duplication. It is only used twice though, so I'm happy to move the logic back if you would prefer.
There was a problem hiding this comment.
And I'll aim to have these sorts of related changes come through in a single PR in the future.
There was a problem hiding this comment.
Ah, right. I forgot about those warnings. I don't have a strong preference either way. I think the real fix is to figure out why we can't use this fixture on the higher levels without warnings. I just added another bullet point to issue #2460 to look into this.
There was a problem hiding this comment.
In this case, it's hypothesis that has issues with the fixture scope. Not sure about other cases.
tests/core/filtering/conftest.py
Outdated
| @pytest.fixture(scope="module") | ||
| def async_create_filter(request): | ||
| return async_partial(return_filter) | ||
| async def async_return_filter(contract=None, args=[]): |
There was a problem hiding this comment.
+1 for @kclowes' comments here
* async existing filter test * async test_filter_against_transaction_logs * async test_filter_against_pending_transactions * async test_contract_get_logs * async test_contract_topic_filters * async test_contract_past_event_filtering * async test_contract_on_event_filtering * async test_contract_data_filters
* async existing filter test * async test_filter_against_transaction_logs * async test_filter_against_pending_transactions * async test_contract_get_logs * async test_contract_topic_filters * async test_contract_past_event_filtering * async test_contract_on_event_filtering * async test_contract_data_filters
* async existing filter test * async test_filter_against_transaction_logs * async test_filter_against_pending_transactions * async test_contract_get_logs * async test_contract_topic_filters * async test_contract_past_event_filtering * async test_contract_on_event_filtering * async test_contract_data_filters
* async existing filter test * async test_filter_against_transaction_logs * async test_filter_against_pending_transactions * async test_contract_get_logs * async test_contract_topic_filters * async test_contract_past_event_filtering * async test_contract_on_event_filtering * async test_contract_data_filters
* async existing filter test * async test_filter_against_transaction_logs * async test_filter_against_pending_transactions * async test_contract_get_logs * async test_contract_topic_filters * async test_contract_past_event_filtering * async test_contract_on_event_filtering * async test_contract_data_filters
What was done?
Async more filter test files
Cute Animal Picture