Skip to content

ci: use Vitest's --changed on non-main branches#1409

Merged
JoshuaKGoldberg merged 21 commits intomainfrom
ci-vitest-changed
Jan 13, 2026
Merged

ci: use Vitest's --changed on non-main branches#1409
JoshuaKGoldberg merged 21 commits intomainfrom
ci-vitest-changed

Conversation

@JoshuaKGoldberg
Copy link
Copy Markdown
Collaborator

@JoshuaKGoldberg JoshuaKGoldberg commented Jan 9, 2026

PR Checklist

Overview

Uses the implementation proposed by @barrymichaeldoyle in #1332 (comment).

Note that all tests are re-run in this PR because it changes configs from main. But future PRs should ™️ nicely re-run only impacted ones.

Co-authored-by: @barrymichaeldoyle

❤️‍🔥

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Jan 9, 2026

⚠️ No Changeset found

Latest commit: f69be29

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link
Copy Markdown

vercel bot commented Jan 9, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
flint Canceled Canceled Jan 12, 2026 11:44pm

@JoshuaKGoldberg
Copy link
Copy Markdown
Collaborator Author

By the way, in order to request review from @flint-fyi/maintenance, I added the team under https://github.com/flint-fyi/flint/settings/access. And then started removing individuals. @barrymichaeldoyle informed me that gives people a kind of scary-looking "you were removed" email. So, I'll wait to keep doing that until people see this message. 😂 sorry if I alarmed you!

@JoshuaKGoldberg JoshuaKGoldberg requested a review from a team January 9, 2026 15:01
path: |
.pnpm-store
node_modules/.vitest
- run: pnpm run build # ensures TS build output for correct test imports
Copy link
Copy Markdown
Member

@auvred auvred Jan 9, 2026

Choose a reason for hiding this comment

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

I wonder if it's worth running only pnpm tsc -b here? As far as I understand, for Vitest tests we don't really need .js outputs, only .d.ts, right? Also, skipping tsdown build here would skip the expensive ATTW check.

cc @lishaduck

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yup, that's correct 👍🏻

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

OH! This just clicked for me. @barrymichaeldoyle I think you might have tried to tell me this when we paired 😂 - sorry if I steamrolled past you.

I had brain lapsed / forgotten that the current Vitest setup goes straight to .ts source. Without that knowledge, it would make sense that pnpm build is needed to get .js outputs. But with it, yeah, tsc -b for .d.ts files makes sense.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ok, right I forgot about vitest-dev/vitest#9353.
I couldn't get ts-patch to work in #1129 without using NODE_OPTIONS. I'll just use it I guess 🤷🏻‍♂️

@JoshuaKGoldberg
Copy link
Copy Markdown
Collaborator Author

Run pnpm tsc -b
Error: packages/flint/src/index.ts(4,29): error TS2307: Cannot find module '@flint.fyi/package-json' or its corresponding type declarations.
Error: Process completed with exit code 2.

☹️ I don't know why this is happening...

@lishaduck
Copy link
Copy Markdown
Member

Ok, so I did fix references. The failure now is that Vitest's setupFiles doesn't respect the resolve.conditions we've set, so it's looking for the .js file which isn't built. Filed vitest-dev/vitest#9429.

That's why we used pnpm build originally: while the tests don't require the sources to be built, they do require ts-patch to be built (so it is still helpful for iterating locally, but you still need an initial build for a clean clone like in CI)

@JoshuaKGoldberg
Copy link
Copy Markdown
Collaborator Author

JoshuaKGoldberg commented Jan 12, 2026

Lovely. Well, if that gets slow over time, we can always look into optimizing later. Thanks! This is very helpful. And it's cool to see issues filed upstream too. 🚀

@JoshuaKGoldberg JoshuaKGoldberg merged commit e5d28b4 into main Jan 13, 2026
9 checks passed
@JoshuaKGoldberg JoshuaKGoldberg deleted the ci-vitest-changed branch January 13, 2026 00:10
@JoshuaKGoldberg
Copy link
Copy Markdown
Collaborator Author

@all-contributors please add @barrymichaeldoyle for infra.

🤖 Beep boop! This comment was added automatically by all-contributors-auto-action.
Not all contributions can be detected from Git & GitHub alone. Please comment any missing contribution types this bot missed.
...and of course, thank you for contributing! 💙

@allcontributors
Copy link
Copy Markdown
Contributor

@JoshuaKGoldberg

I've put up a pull request to add @barrymichaeldoyle! 🎉

JoshuaKGoldberg pushed a commit that referenced this pull request Jan 13, 2026
Adds @barrymichaeldoyle as a contributor for infra.

This was requested by JoshuaKGoldberg [in this
comment](#1409 (comment))

---------

Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>
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.

🛠 Tooling: Only run "impacted by" tests for PRs in CI (--changed)

4 participants