Skip to content

Conversation

@kclowes
Copy link
Collaborator

@kclowes kclowes commented Apr 23, 2025

What was wrong?

Tests were xfailing but should have been passing. Highlights the need to specify which exception tests expect.

How was it fixed?

Added a new AsyncStaticENS class for tests instead of using StaticENS which had a sync address method.

Todo:

  • Clean up commit history
  • Add or update documentation related to these changes
  • Add entry to the release notes

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

@kclowes kclowes marked this pull request as ready for review April 23, 2025 19:53
@kclowes kclowes requested review from Copilot and fselmo April 23, 2025 19:53
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enables the asynchronous ENS tests to run by introducing a new AsyncStaticENS class and updating test configurations.

  • Removed xfail markers from async ENS tests
  • Introduced AsyncStaticENS with an async address method
  • Updated ens_addresses to use AsyncStaticENS when running asynchronously

Reviewed Changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.

File Description
web3/_utils/module_testing/eth_module.py Removed xfail markers from async ENS tests to allow proper test execution
web3/_utils/ens.py Added AsyncStaticENS and updated ens_addresses to support async ENS resolution
Files not reviewed (1)
  • newsfragments/3675.internal.rst: Language not supported

@kclowes kclowes requested review from pacrob and reedsa April 23, 2025 19:53
Copy link
Contributor

@reedsa reedsa left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Collaborator

@fselmo fselmo left a comment

Choose a reason for hiding this comment

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

nice catch!

@kclowes kclowes merged commit 366170d into ethereum:main Apr 24, 2025
85 checks passed
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.

3 participants