Teach the Ghostwriter about @staticmethod and @classmethod methods #3354

Cheukting · 2022-05-19T22:41:40Z

In this PR:

makes the CLI understands module_a.ClassA.function
make sure roundtrip works and not get confused with functions in other classes
improve on 'magic discovery' and make it works with classes
- Discover class/static methods when input just module
- Accept class as input

closes #3318

Zac-HD

Off to a good start 🙂

hypothesis-python/src/hypothesis/extra/cli.py

Cheukting · 2022-05-22T20:19:44Z

@Zac-HD I have a question since a class is also callable, I think in the previous logic, if you put a class as input it will write a test as if it is a function but if you put the module as input it will not be discovered. Do you mind clarifying what is the desired behaviour for the class itself?

Zac-HD · 2022-05-23T02:22:30Z

I think the previous behavior is fine here; but I'm pretty happy going with whatever is easiest to implement - the Ghostwriter is really aimed at making it easy to get started, so I care a lot about handling functions and very little about classes (because just creating an instance is rarely a useful test).

Cheukting · 2022-05-23T06:25:12Z

If that is the case, I would suggest not create a test for the class. If someone input the class, the magic will try to discover if there is any static/ class method for tests.

…

On Mon, 23 May 2022 at 03:22, Zac Hatfield-Dodds ***@***.***> wrote: I think the previous behavior is fine here; but I'm pretty happy going with whatever is easiest to implement - the Ghostwriter is really aimed at making it easy to get started, so I care a lot about handling functions and very little about classes (because just creating an instance is rarely a useful test). — Reply to this email directly, view it on GitHub <#3354 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AG3N26OV3YRKR53FLSOSFUDVLLTXHANCNFSM5WNU5QIQ> . You are receiving this because you authored the thread.Message ID: ***@***.***>

Cheukting · 2022-05-23T13:00:22Z

write an expected-output test so that we can see that the results are qualitatively good

confirm that we can write {fuzz,roundtrip,equivalence} tests as well as magic

Zac-HD

Code comments below; I also took a couple of attempts and eventually (after looking at the Sphinx source code!) managed to get the classmethod and staticmethod cross-references to link properly 😅

hypothesis-python/tests/ghostwriter/test_ghostwriter.py

hypothesis-python/tests/ghostwriter/test_ghostwriter_cli.py

Zac-HD · 2022-05-27T07:51:49Z

hypothesis-python/tests/ghostwriter/test_expected_output.py

+        pytest.param(
+            ("xml_etree_ElementTree_py38", ghostwriter.magic(xml.etree.ElementTree)),
+            marks=[
+                pytest.mark.skipif(
+                    sys.version_info[:2] != (3, 8),
+                    reason="output generated with Python 3.8",
+                )
+            ],
+        ),
+        pytest.param(
+            ("xml_etree_ElementTree_py310", ghostwriter.magic(xml.etree.ElementTree)),


I'd try defining a class in this module, and use that - it works for the with_docstring function, for example.

hypothesis-python/src/hypothesis/extra/ghostwriter.py

Zac-HD · 2022-05-31T08:32:59Z

Looking good! I think the 3.11 failure is a genuine problem somewhere in either Pytest or maybe Hypothesis (pre-dating thia PR), so I'll aim to investigate and fix that in the next few days.

And thanks again for sticking with this long process 🤩

Zac-HD · 2022-06-01T07:44:30Z

Yep, the 3.11 crash was an upstream CPython-nightly issue affecting Pytest, which has since been fixed: pytest-dev/pytest#10008.

Zac-HD · 2022-06-02T05:01:30Z

Will closing and reopening unstick CI? Let's find out!

Zac-HD · 2022-06-02T05:10:28Z

Welp, looks like GitHub Actions is pretty seriously degraded at the moment... I'll restart again when it recovers 🙃

Zac-HD

We're very close now - just some minor comments and a request for, well, more comments on the tests 🙂

hypothesis-python/src/hypothesis/extra/ghostwriter.py

Zac-HD · 2022-06-02T05:17:12Z

hypothesis-python/tests/ghostwriter/test_ghostwriter_cli.py

+        ("hypothesis.strategies.builds", lambda: fuzz(hypothesis.strategies.builds)),
+        # Discover methods in class
+        ("hypothesis.errors.StopTest", lambda: magic(StopTest)),
        # Imports submodule (importlib.import_module passes; __import__ fails)
-        ("hypothesis.errors.StopTest", lambda: fuzz(StopTest)),
+        ("hypothesis.strategies", lambda: magic(hypothesis.strategies)),


What are each of these new tests checking for? Is that comment about import semantics still correct, even though we've changed what we're testing?

I'd prefer to just swap magic(StopTest) for fuzz(StopTest), but if we're adding more we also need to add corresponding comments.

Because there's no classmethod and staticmethod in StopTest so it didn't work for fuzz before (the CLI will show "nothing to test" and fuzz will have an error) this is due to the class itself was not tested in the previous generation of this PR

What are each of these new tests checking for?

I think the comment above kinda indicates something? for the magic(hypothesis.strategies) it is a submodule and for magic(StopTest) it discovers the methods in class (though I think it's a bad example as there's nothing in StopTest I believe)

Zac-HD · 2022-06-02T05:20:37Z

hypothesis-python/tests/ghostwriter/test_ghostwriter_cli.py

+        (
+            "my_func",
+            "XX",
+            "Found the 'mycode' module and 'my_func' attribute, but it doesn't have a 'XX' attribute.",
+        ),


You could compress this parametrize down to just ("my_func", " and 'my_func' attribute", "attribute") - with a bit more string formatting. That would make it much more compact, which I think would be easier to read.

Do you mean the error message? Can't quite get what you mean here 🙃

hypothesis-python/tests/ghostwriter/test_expected_output.py

Cheukting · 2022-06-02T07:59:19Z

@Zac-HD Looks like GH action CI is broken.... https://github.com/HypothesisWorks/hypothesis/runs/6704398671?check_suite_focus=true#step:1:37

Zac-HD

I'm back from a busy weekend, and we're completely done and ready to merge! Congrats Cheuk; it turned out to be a long and twisty one but a great contribution in the end 😁:tada:

Cheukting · 2022-06-07T06:58:05Z

I'm back from a busy weekend, and we're completely done and ready to merge! Congrats Cheuk; it turned out to be a long and twisty one but a great contribution in the end 😁🎉

Thank you for your patient guidance @Zac-HD

Cheukting added 6 commits May 19, 2022 23:36

improve CLi for staticmethod

45e903f

shed

1550b15

fix classname error

5709757

Add matches

4edfd79

Adding from err

1032296

Add test and fix err

d2179a9

Zac-HD reviewed May 21, 2022

View reviewed changes

hypothesis-python/src/hypothesis/extra/cli.py Outdated Show resolved Hide resolved

hypothesis-python/src/hypothesis/extra/cli.py Outdated Show resolved Hide resolved

hypothesis-python/src/hypothesis/extra/cli.py Outdated Show resolved Hide resolved

Cheukting added 4 commits May 21, 2022 09:54

remove err on line 98

7aa6a69

check if func_class is class

b101eba

add test

ba482b0

magic scan and include methods in classes

c08d3c5

Cheukting force-pushed the ghostwriter_imrpovement branch from 6abd9c3 to c08d3c5 Compare May 21, 2022 17:59

Cheukting added 7 commits May 21, 2022 20:06

adding magic discovery test

f6b170f

adding roundtrip test

5b6c402

add more roundtrip test

f614a7f

remove pkg check for class attributes

1fc4e6b

fix typo

7d9202e

update output, include class/static methods in tests

c1d253c

cli can accept class as input

bb1b526

Cheukting force-pushed the ghostwriter_imrpovement branch from 6b91652 to bb1b526 Compare May 23, 2022 06:46

Cheukting added 6 commits May 23, 2022 07:57

refactoring

1ad5f09

fuzz only works on functions

4c43092

add more tests

8b9a226

single ghostwriters does not works on class itself

6e7f60b

fix test using a class, use method instead

7559c85

Adding expected output test

8dc4dac

Cheukting and others added 6 commits May 26, 2022 07:46

delete dead code

43c4807

using loop to test

06c6e96

make mypy happy again

31915da

release.rst

a85fd3e

Update RELEASE.rst

ff3f06e

Update RELEASE.rst

230d85e

Zac-HD reviewed May 27, 2022

View reviewed changes

Cheukting added 4 commits May 31, 2022 08:09

remove duplicate cause

789fb00

update type annotation

158bb6e

condense more with round_trip_code

a14c416

Fix formating

4502059

xml.etree.ElementTree stable after 3.9

1a6e42a

Zac-HD closed this Jun 2, 2022

Zac-HD reopened this Jun 2, 2022

Zac-HD reviewed Jun 2, 2022

View reviewed changes

use list instead of copy

9364727

Cheukting added 2 commits June 2, 2022 09:49

swap testing with xml to custom class

42dfdb7

shed

860eaff

Cheukting force-pushed the ghostwriter_imrpovement branch from c0687b2 to d6a999a Compare June 5, 2022 09:14

type annotation

dfee405

Cheukting force-pushed the ghostwriter_imrpovement branch from d6a999a to dfee405 Compare June 5, 2022 12:39

Tighten up tests a bit

2c615d7

Zac-HD approved these changes Jun 7, 2022

View reviewed changes

Zac-HD merged commit 208b862 into HypothesisWorks:master Jun 7, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Teach the Ghostwriter about @staticmethod and @classmethod methods #3354

Teach the Ghostwriter about @staticmethod and @classmethod methods #3354

Cheukting commented May 19, 2022 •

edited

Loading

Zac-HD left a comment

Cheukting commented May 22, 2022

Zac-HD commented May 23, 2022

Cheukting commented May 23, 2022 via email

Cheukting commented May 23, 2022 •

edited

Loading

Zac-HD left a comment

Zac-HD May 27, 2022

Zac-HD commented May 31, 2022

Zac-HD commented Jun 1, 2022

Zac-HD commented Jun 2, 2022

Zac-HD commented Jun 2, 2022

Zac-HD left a comment

Zac-HD Jun 2, 2022

Cheukting Jun 2, 2022 •

edited

Loading

Cheukting Jun 2, 2022

Zac-HD Jun 2, 2022

Cheukting Jun 2, 2022

Cheukting commented Jun 2, 2022

Zac-HD left a comment

Cheukting commented Jun 7, 2022

Teach the Ghostwriter about @staticmethod and @classmethod methods #3354

Teach the Ghostwriter about @staticmethod and @classmethod methods #3354

Conversation

Cheukting commented May 19, 2022 • edited Loading

Zac-HD left a comment

Choose a reason for hiding this comment

Cheukting commented May 22, 2022

Zac-HD commented May 23, 2022

Cheukting commented May 23, 2022 via email

Cheukting commented May 23, 2022 • edited Loading

Zac-HD left a comment

Choose a reason for hiding this comment

Zac-HD May 27, 2022

Choose a reason for hiding this comment

Zac-HD commented May 31, 2022

Zac-HD commented Jun 1, 2022

Zac-HD commented Jun 2, 2022

Zac-HD commented Jun 2, 2022

Zac-HD left a comment

Choose a reason for hiding this comment

Zac-HD Jun 2, 2022

Choose a reason for hiding this comment

Cheukting Jun 2, 2022 • edited Loading

Choose a reason for hiding this comment

Cheukting Jun 2, 2022

Choose a reason for hiding this comment

Zac-HD Jun 2, 2022

Choose a reason for hiding this comment

Cheukting Jun 2, 2022

Choose a reason for hiding this comment

Cheukting commented Jun 2, 2022

Zac-HD left a comment

Choose a reason for hiding this comment

Cheukting commented Jun 7, 2022

Cheukting commented May 19, 2022 •

edited

Loading

Cheukting commented May 23, 2022 •

edited

Loading

Cheukting Jun 2, 2022 •

edited

Loading