Skip to content

Conversation

@AlexWaygood
Copy link
Member

@AlexWaygood AlexWaygood commented Jan 14, 2023

We have various bits of logic scattered across the test suite to parse METADATA.toml files. This is a bug magnet: see e.g. #9532 (comment). (Failing CI run here: https://github.com/python/typeshed/actions/runs/3918898414/jobs/6699564673.) By centralizing the logic all in one place, we reduce the risk of bugs in our tests, and also improve type safety.

This PR centralizes METADATA.toml-parsing logic into a new file, tests/parse_metadata.py.

return modules


def check_metadata() -> None:
Copy link
Collaborator

@Avasam Avasam Jan 14, 2023

Choose a reason for hiding this comment

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

At this point, this feels redundant since other tests will fail anyway. 😅
Unless you wanna use it as a smoke test.

Copy link
Member Author

@AlexWaygood AlexWaygood Jan 14, 2023

Choose a reason for hiding this comment

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

I get what you mean, but the other tests will assert the correctness of the METADATA.toml files "by accident". I feel like it's useful to have a test that asserts the correctness of the METADATA.toml files "on purpose"? That way we still have this guaranteed to be tested in CI, even if some other tests no longer need to get stuff from METADATA.toml files in the future, for whatever reason.

Copy link
Collaborator

@Avasam Avasam Jan 14, 2023

Choose a reason for hiding this comment

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

Yeah that's what I meant by a smoke test (an easy test that says, if this fails, everything else will fail anyway, so it's easy to find the source of the error). Just wanted to know if it was on purpose :)

@Avasam
Copy link
Collaborator

Avasam commented Jan 17, 2023

@AlexWaygood
Copy link
Member Author

What about the [mypy-tests] sections? https://github.com/python/typeshed/pull/9534/files#diff-b42eb36bcaa2d1a163ed1c202e81cffd058915ca695b660d49c636b3a5b56857R199-R216 (which doesn't seem to be currently used anywhere?)

I believe that if somebody did try to add a per-stub config section for mypy, check_consistent.py would already complain about it. And, as you say, it's not currently used anywhere. I'd prefer not to touch it for this PR.

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.

2 participants