Skip to content

refactor(ts-plugin): migrate tests to typescript#16417

Merged
ematipico merged 4 commits intowithastro:mainfrom
ocavue-forks:ocavue-ts-test-ts-plugin3
Apr 21, 2026
Merged

refactor(ts-plugin): migrate tests to typescript#16417
ematipico merged 4 commits intowithastro:mainfrom
ocavue-forks:ocavue-ts-test-ts-plugin3

Conversation

@ocavue
Copy link
Copy Markdown
Contributor

@ocavue ocavue commented Apr 20, 2026

Part of #16241

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Apr 20, 2026

⚠️ No Changeset found

Latest commit: 3a9e685

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.

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

mocha: {
ui: 'tdd',
timeout: 20000,
require: ['tsx'],
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We use @vscode/test-cli to run the test, which is based on mocha. I need require: ['tsx'] in order to run typescript test files in mocha.

@ocavue ocavue marked this pull request as ready for review April 20, 2026 19:57
@ocavue ocavue force-pushed the ocavue-ts-test-ts-plugin3 branch from f349f94 to 54b8b62 Compare April 20, 2026 20:21
Copy link
Copy Markdown
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

I'll keep this for last. The language tool is special I haven't taken that into account for the migration.

@ocavue
Copy link
Copy Markdown
Contributor Author

ocavue commented Apr 21, 2026

I'll keep this for last. The language tool is special I haven't taken that into account for the migration.

Surely. I know this is not in your migration plan. Language tool (vscode) uses mocha, unlike other tests that use node:test and playwright. I simply want to get rid of all *.test.js files in the repo using this chance.

@ematipico
Copy link
Copy Markdown
Member

@Princesseuh I would love your review here

@Princesseuh
Copy link
Copy Markdown
Member

Princesseuh commented Apr 21, 2026

I don't think this test actually runs right now, IIRC it was super flaky when I moved it to the monorepo so I ended up skipping it. I can't seem to find it in the CI logs either, so guessing this is still the case

@ocavue
Copy link
Copy Markdown
Contributor Author

ocavue commented Apr 21, 2026

Oh I forget to check CI log. The test runs fine on my local Mac environment.

@github-actions github-actions Bot added the 🚨 action Modifies GitHub Actions label Apr 21, 2026
@ocavue
Copy link
Copy Markdown
Contributor Author

ocavue commented Apr 21, 2026

@Princesseuh I've added back the ts-plugin test to CI in 3a9e685.

Currently, this test only runs on Linux. The last push triggered two runs, one on Ubuntu with Node 20 and another on Ubuntu with Node 22. Both runs were successful.

Since you mentioned that this test is flaky, I ensured that this test won't break the CI. If the test fails, it will just print a warning message that can be seen in the GitHub Action UI using this syntax. We can merge the CI change and monitor whether this test is still flaky.

Copy link
Copy Markdown
Member

@Princesseuh Princesseuh left a comment

Choose a reason for hiding this comment

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

This looks fine to me, but I'll let @ematipico decide if he thinks the flaky warning is ok or not!

Copy link
Copy Markdown
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

Thank you both. Let's keep it as is. If it starts bothering CI, we can silent them again

@ematipico ematipico merged commit 0c0ae11 into withastro:main Apr 21, 2026
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🚨 action Modifies GitHub Actions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants