erictraut / cpython

The Python programming language
https://www.python.org/
Other
2 stars 0 forks source link

Write plenty of unit tests #6

Open gvanrossum opened 1 year ago

gvanrossum commented 1 year ago

This large, complex PEP requires a lot of unit tests. Hopefully @erictraut's original prototype had some that we could convert, but I suspect we need a lot more.

A problem is that the unit tests can't test the semantics until those are implemented. But at least we can have tests right now that verify that all the various forms of syntax are supported (and that certain near-correct but invalid inputs are actually rejected with reasonable SyntaxError messages).

erictraut commented 1 year ago

I have a pretty good start on tests for both generic classes and functions and generic type aliases. They cover attempt to cover both the error non-error paths.

I just noticed that the existing tests assume the scoping rules of the older spec. Some of the unit tests will need to be updated to comply with the approved version of the spec, but that should be relatively easy.

gvanrossum commented 1 year ago

Okay, this is a good start, though around 500 lines of tests is pretty modest for such a big new feature.

I just noticed that the existing tests assume the scoping rules of the older spec. Some of the unit tests will need to be updated to comply with the approved version of the spec, but that should be relatively easy.

That's a bit worrisome, as someone working on the new implementation might assume that the tests specify the desired behavior. I'm glad we've been warned!

JelleZijlstra commented 1 year ago

I think it will be helpful to get a good set of tests into the branch that we'll eventually submit as a CPython PR (type_param_syntax4 I believe), even if the tests don't all pass on the branch yet. That way, we at least know what to work towards.

erictraut commented 1 year ago

I updated the unit tests in the type_param_syntax4 branch so they comply with the latest PEP 695 spec. I can't run the tests on the branch currently because the PEP 695 implementation is incomplete, but I validated the unit test against the pyright implementation, which is up to date with the latest spec.

JelleZijlstra commented 1 year ago

I am looking at this test in your branch:

    def test_traditional_02(self):
        code = textwrap.dedent("""\
                from typing import TypeVar
                S = TypeVar("S")
                class ClassA[T](dict[T, S]): ...
            """
        )

        with self.assertRaisesRegex(TypeError, r"Some type variables \(~S\) are not listed in Generic\[T\]"):
            exec(code, {}, {})

I agree that type checkers should flag this as an error, but I don't think a runtime error makes sense here. Where would you envision the runtime catching this?

erictraut commented 1 year ago

Good catch, that should not be a runtime error. Do you want to fix the test, or would you like me to do it?

JelleZijlstra commented 1 year ago

I can fix it, I am working on some other test changes.