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

Figure out a way to have test cases for third-party stubs #8569

Closed
AlexWaygood opened this issue Aug 19, 2022 · 12 comments · Fixed by #8700
Closed

Figure out a way to have test cases for third-party stubs #8569

AlexWaygood opened this issue Aug 19, 2022 · 12 comments · Fixed by #8700
Assignees
Labels
project: infrastructure typeshed build, test, documentation, or distribution related

Comments

@AlexWaygood
Copy link
Member

We currently only have test cases for our stdlib stubs, and don't currently have the infrastructure set up to allow us to add test cases for any of our third-party stubs. This has come up a few times now, so it would be good to figure out a way to get this to work.

(This issue relates to the test cases in our test_cases directory. Currently we test these samples with mypy and pyright.)

@AlexWaygood AlexWaygood added the project: infrastructure typeshed build, test, documentation, or distribution related label Aug 19, 2022
@AlexWaygood AlexWaygood self-assigned this Aug 19, 2022
@AlexWaygood
Copy link
Member Author

AlexWaygood commented Aug 19, 2022

As @srittau wrote in #8505 (comment):

Ideally, we'd put third-party tests into stubs/*/@tests, not in test_cases.

@kkirsche
Copy link
Contributor

To get us started on this, I think it would make sense to outline a few stubs that would be good candidates for this so that we can leverage those stubs when prototyping or suggesting approaches.

@sobolevn
Copy link
Member

In django-stubs we run type checking on django tests.
It is not ideal at all, but our project is quite old at this point, so we didn't have much options back in the days.

Link: https://github.com/typeddjango/django-stubs/blob/master/scripts/typecheck_tests.py

@AlexWaygood
Copy link
Member Author

Some updates on my plan for this:

@Akuli
Copy link
Collaborator

Akuli commented Aug 29, 2022

Most of the requests problems were introduced when mypy_primer was broken for third-party stubs.

@AlexWaygood
Copy link
Member Author

Most of the requests problems were introduced when mypy_primer was broken for third-party stubs.

Yeah, mypy_primer is and always will be our best tool for catching regressions. The test_cases directory will always be a secondary tool.

@kkirsche
Copy link
Contributor

I can help write tests for some of aspects of Flask-SQLalchemy and SQLAlchemy, depending on the coverage goals. To the question of what it is that we're testing, what should these tests actually be covering?

What I mean by that is meant to be the specifics not the ideological discussion covered in #8505, e.g. are we testing that the behavior is functional (so using an in-memory or external database service to verify both the actions and the types) and the types pass, or only that the types pass for idiomatic use cases (essentially acting as a form of documentation while providing a bit more security in updates)?

If using external services, such as the database in the case above or an external service's URL for requests, what external resources does the team want to use for these, as this has implications on test stability if there are downstream outages or issues?

@AlexWaygood
Copy link
Member Author

AlexWaygood commented Sep 1, 2022

I can help write tests for some of aspects of Flask-SQLalchemy and SQLAlchemy

Thanks for the offer! I appreciate it.

depending on the coverage goals. To the question of what it is that we're testing, what should these tests actually be covering?

As stated in the test_cases README, we're not really going for 100% code coverage at all with these test cases; it's just really meant to be for exceptional cases where the stubs are quite tricky to get right, and/or where we've run into specific difficulties in the past. But if you have specific proposals for certain areas of SQLAlchemy or Flask-SQLAlchemy that you think could do with some test cases, we can discuss it once we've got the infrastructure set up.

If using external services, such as the database in the case above or an external service's URL for requests, what external resources does the team want to use for these, as this has implications on test stability if there are downstream outages or issues?

The test cases being discussed in this issue shouldn't need access to any external services. The code in these test cases is never actually run; we just run the type checkers on the code so that we can assert that type-checking errors are emitted in the correct places. This isn't a comprehensive way of testing stubs, but it's not meant to be; if we want to discuss more comprehensive ways of testing stubs, we should do that elsewhere :)

@kkirsche
Copy link
Contributor

kkirsche commented Sep 2, 2022

OK, cool.

So in terms of infrastructure, what you mean by that is:

  1. Update or create a new github action to run the tests
  2. A utility (either extension of an existing script or a new script) to run the new tests
  3. Creation of at least one set of tests for a library to validate the infrastructure works as expected

The above assumes that we'd use stubs/*/@tests for the files to be tested. Potentially as a sub-directory to avoid mixing test files with configuration files, as would occur due to things like stubtest_allowlist.txt files and whatnot. If agreed to in the scoping, potentially separate into @tests/config and @tests/usage.

Is there anything I'm missing / am I over simplifying what's required?

@AlexWaygood
Copy link
Member Author

So in terms of infrastructure, what you mean by that is:

  1. Update or create a new github action to run the tests
  2. A utility (either extension of an existing script or a new script) to run the new tests
  3. Creation of at least one set of tests for a library to validate the infrastructure works as expected

That pretty much covers it, yeah. I've assigned this issue to me to indicate that I'd like to work on these steps :)

@AlexWaygood
Copy link
Member Author

Is there anything I'm missing / am I over simplifying what's required?

One complicating factor is:

If we can figure out the right way to organise our pyrightconfig.json files, it should really be pretty easy to have pyright's reportUnnecessaryTypeIgnoreComment setting enabled by default for the whole test_cases directory. If we want to have third-party stubs in stubs/*/@tests/test_cases directories, however, rather than in the top-level test_cases directory, then having that pyright setting enabled by default for all our third-party test cases might be a little tricky.

@AlexWaygood
Copy link
Member Author

AlexWaygood commented Sep 7, 2022

Okay, here's a PR! Reviews welcome:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
project: infrastructure typeshed build, test, documentation, or distribution related
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants