-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Auto-accept snapshot changes as part of typeshed-sync PRs #20892
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
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
sharkdp
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome idea.
59dcd49 to
b9c2375
Compare
b9c2375 to
6b62346
Compare
|
I kicked off a test run here using this branch: https://github.com/astral-sh/ruff/actions/runs/18533177734 |
11f005f to
e7b81ae
Compare
e7b81ae to
4e452e5
Compare
It took a few iterations but I think it all looks good now: |
…rable * origin/main: Don't use codspeed or depot runners in CI jobs on forks (#20894) [ty] cache Type::is_redundant_with (#20477) Fix run-away for mutually referential instance attributes (#20645) [ty] Limit shown import paths to at most 5 unless ty runs with `-v` (#20912) [ty] Use field-specifier return type as the default type for the field (#20915) [ty] Do not assume that `field`s have a default value (#20914) [ty] Fix match pattern value narrowing to use equality semantics (#20882) Update setup instructions for Zed 0.208.0+ (#20902) Move TOML indent size config (#20905) [syntax-errors]: implement F702 as semantic syntax error (#20869) [ty] Heterogeneous unpacking support for unions (#20377) [ty] refactor `Place` (#20871) Auto-accept snapshot changes as part of typeshed-sync PRs (#20892) [`airflow`] Add warning to `airflow.datasets.DatasetEvent` usage (`AIR301`) (#20551) [`flake8-pyi`] Fix operator precedence by adding parentheses when needed (`PYI061`) (#20508) [`pyupgrade`] Fix false negative for `TypeVar` with default argument in `non-pep695-generic-class` (`UP046`) (#20660) Update parser snapshots (#20893) Fix syntax error false positives for escapes and quotes in f-strings (#20867)
…nt-sets * dcreager/non-non-inferable: Don't use codspeed or depot runners in CI jobs on forks (#20894) [ty] cache Type::is_redundant_with (#20477) Fix run-away for mutually referential instance attributes (#20645) [ty] Limit shown import paths to at most 5 unless ty runs with `-v` (#20912) [ty] Use field-specifier return type as the default type for the field (#20915) [ty] Do not assume that `field`s have a default value (#20914) [ty] Fix match pattern value narrowing to use equality semantics (#20882) Update setup instructions for Zed 0.208.0+ (#20902) Move TOML indent size config (#20905) [syntax-errors]: implement F702 as semantic syntax error (#20869) [ty] Heterogeneous unpacking support for unions (#20377) [ty] refactor `Place` (#20871) Auto-accept snapshot changes as part of typeshed-sync PRs (#20892) [`airflow`] Add warning to `airflow.datasets.DatasetEvent` usage (`AIR301`) (#20551) [`flake8-pyi`] Fix operator precedence by adding parentheses when needed (`PYI061`) (#20508) [`pyupgrade`] Fix false negative for `TypeVar` with default argument in `non-pep695-generic-class` (`UP046`) (#20660) Update parser snapshots (#20893) Fix syntax error false positives for escapes and quotes in f-strings (#20867)
|
It doesn't look this worked, but I don't really understand why :/ The typeshed sync workflow last night ran the full test suite, and I can see snapshot tests failing as part of the run. But no snapshot changes were accepted afterwards; it just says
https://github.com/astral-sh/ruff/actions/runs/19381406943/job/55460969374 I've tried running the same command locally with the same environment variables set, and it always results in snapshots being updated at the end of the run. Not sure what's up here. |
Summary
Some of our ty snapshots (in particular, several in the
ty_idecrate that relate to autocompletions) are very sensitive to changes in typeshed. If typeshed changes the line number a class or function is defined on, that often leads to the snapshots inty_ideneeding to be updated manually before a typeshed-sync PR can be merged, which is a bit of a pain.This PR adds a step to the
sync_typeshed.yamlworkflow that checks for changes to snapshots and automatically accepts them all before the PR is made. This should hopefully reduce the number of changes we need to make manually to sync-typeshed PRs. (We'll still need to review these PRs carefully to check that all the changes to the snapshots are desirable, but I still think that this will probably save us time overall.) The step is allowed to fail; if checking for changes to snapshots produces a nonzero exit code, the sync-typeshed PR should still be filed.The
cargo instadocs are here: https://insta.rs/docs/cli/Test Plan
I:
git checkout dc64c086336148c57db14060c86f448351710b2e -- crates/ty_python_semantic/resources/mdtest/snapshots. This reverted all the snapshot changes from [ty] Filter outrevealed-typeandundefined-revealdiagnostics from mdtest snapshots #20820, a PR which resulted in lots of snapshot changes -- multiple snapshot changes per mdtest file, in some cases.cargo insta test --accept. I observed that this command reverted all changes from step (1).