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

Implement get_resource_reader method in rewrite's loader #26

Merged
merged 1 commit into from
Oct 5, 2024

Conversation

giladmoav
Copy link
Contributor

The issue in #25 is that AssertionRewritingHook doesn't implement get_resource_reader (from PEP302), I implemented the method like in pytest

Comment on lines 214 to 220
def get_resource_reader(self, name: str) -> importlib.abc.TraversableResources:
from types import SimpleNamespace
if sys.version_info < (3, 11):
from importlib.readers import FileReader
else:
from importlib.resources.readers import FileReader
return FileReader(SimpleNamespace(path=self._rewritten_names[name]))
Copy link

Choose a reason for hiding this comment

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

Is there a reason you didn't use the latest code from pytest:

    if sys.version_info >= (3, 10):
        if sys.version_info >= (3, 12):
            from importlib.resources.abc import TraversableResources
        else:
            from importlib.abc import TraversableResources

        def get_resource_reader(self, name: str) -> TraversableResources:
            if sys.version_info < (3, 11):
                from importlib.readers import FileReader
            else:
                from importlib.resources.readers import FileReader

            return FileReader(types.SimpleNamespace(path=self._rewritten_names[name]))

https://github.com/pytest-dev/pytest/blob/main/src/_pytest/assertion/rewrite.py

@LiadOz
Copy link

LiadOz commented May 29, 2024

@giladmoav Can you try to write a test for it?

@giladmoav giladmoav force-pushed the fix-loader-issue branch 2 times, most recently from 11f0e99 to 09ed945 Compare May 29, 2024 11:30
@LiadOz
Copy link

LiadOz commented May 30, 2024

@giladmoav looks good
@ayalash Can you please review these changes?

@oren0e
Copy link

oren0e commented Sep 17, 2024

@ayalash Hi, any progress on that?

@ayalash
Copy link
Collaborator

ayalash commented Oct 5, 2024

Thanks @giladmoav & @LiadOz !

@ayalash ayalash merged commit db24c60 into getslash:develop Oct 5, 2024
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.

4 participants