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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inaccurate filename due to test class metaprogramming that doesn't set __file__ and __name__ #125467

Open
ezyang opened this issue May 3, 2024 · 3 comments
Assignees
Labels
module: devx Related to PyTorch contribution experience (HUD, pytorchbot) module: tests Issues related to tests (not the torch.testing module) triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module

Comments

@ezyang
Copy link
Contributor

ezyang commented May 3, 2024

馃悰 Describe the bug

I noticed that occasionally the "To execute this test" doesn't have complete information. For example:

2024-05-02T20:42:53.9094051Z To execute this test, run the following from the base repo dir:
2024-05-02T20:42:53.9094398Z      python test_compiled_autograd.py -k test_accumulate_grad_accuracy

but sometimes you just having testing.py, you don't even have a useful name.

This is usually always because the __file__ on the test class is not set appropriately.

Here is a sample fix diff:

diff --git a/test/inductor/test_compiled_autograd.py b/test/inductor/test_compiled_autograd.py
index 2a94e4cee5b..14aa16c059b 100644
--- a/test/inductor/test_compiled_autograd.py
+++ b/test/inductor/test_compiled_autograd.py
@@ -1535,11 +1535,13 @@ def wrap_test_class(orig_cls):
         elif name.startswith("test_"):
             dct[name] = make_wrapped(fn)
 
-    return type(
+    cls = type(
         orig_cls.__name__ + "WithCompiledAutograd",
         orig_cls.__bases__,
         dct,
     )
+    cls.__file__ = __file__
+    return cls
 
 
 # These groups of tests aren't supported yet

we have to apply these to all test metaprograms. Maybe we can have a test to check __file__ is actually set. Note that setting __file__ to __file__ is not always the right thing to do.

Versions

main

cc @mruberry @ZainRizvi @kit1980 @huydhn @clee2000

@janeyx99 janeyx99 added module: tests Issues related to tests (not the torch.testing module) triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module module: devx Related to PyTorch contribution experience (HUD, pytorchbot) labels May 3, 2024
@janeyx99
Copy link
Contributor

janeyx99 commented May 3, 2024

cc @jbschlosser

@jbschlosser
Copy link
Contributor

jbschlosser commented May 3, 2024

From what I can tell, the __file__ attribute is only intended to be present on module objects and not type objects. For a type object, I think inspect.getfile(test_class_type) (which is used for the repro command) will check the __file__ attribute of the module referenced by the type's __module__ attribute.

For this particular example, the __module__ attribute is inherited from the parent class by the metaprogrammed class via dct:

return type(
orig_cls.__name__ + "WithCompiledAutograd",
orig_cls.__bases__,
dct,
)

This results in the wrong path being reported by the repro command.

>>> sys.modules[TestAutogradWithCompiledAutograd.__module__].__file__
'/home/jbschlosser/pytorch/test/test_autograd.py'
>>> inspect.getfile(TestAutogradWithCompiledAutograd)
'/home/jbschlosser/pytorch/test/test_autograd.py'

I'd suggest ensuring __module__ is correct for the metaprogrammed class, but idk if this is possible since the file it's defined in may not be related to a loaded module.

@ezyang
Copy link
Contributor Author

ezyang commented May 3, 2024

Huh, I guess it checks __file__ anyway. Fixing __module__ should work. Using the original cls module seems like the ticket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module: devx Related to PyTorch contribution experience (HUD, pytorchbot) module: tests Issues related to tests (not the torch.testing module) triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module
Projects
Status: Cold Storage
Development

No branches or pull requests

4 participants