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

Tests: Update to TypeScript #316

Merged
merged 21 commits into from
Oct 27, 2021
Merged

Tests: Update to TypeScript #316

merged 21 commits into from
Oct 27, 2021

Conversation

joryphillips
Copy link
Contributor

This PR updates test infrastructure to @web/test-runner per #240 and converts all test files to TypeScript.

Some notes on methodology:

  • Tried to do the minimum I could and did not think too hard about how tests might be better, except where I ran into problems (see below)
  • Removed excess requestAnimationFrames (via cycle() or nextFrame()) where they were clearly duplicative or superfluous. However, I left some in places where they might not be strictly needed for a passing test, but seem good for testing: for instance, after a useState or similar value is updated.
  • Needed to make some minor typing changes in source code to make TS happy for some tests.
  • Also needed to import from src instead of compiled code for this to work. Otherwise we get Error: Invalid template provided - string or lit-html TemplateResult is supported
  • useEffect and useController tests are where I hit the biggest issues (which were not that big). I added comments or updated code to reflect my findings & understanding of intended direction.
  • There were a couple failing things in types-test.ts; I added changes & comments to highlight these. After some investigation of those issues and landing this PR, we can probably delete that file.
  • I think after we update the build system to Rollup we can remove the .js from imports in those test files. I needed to add those extensions for make to work correctly.

@netlify
Copy link

netlify bot commented Oct 8, 2021

✔️ Deploy Preview for hauntedhooks ready!

🔨 Explore the source changes: 455d027

🔍 Inspect the deploy log: https://app.netlify.com/sites/hauntedhooks/deploys/61799c171d372200083bb0cf

😎 Browse the preview: https://deploy-preview-316--hauntedhooks.netlify.app

@matthewp
Copy link
Owner

matthewp commented Oct 8, 2021

Yes! Thank you so much! I'll leave it to @bennypowers to give their blessing as I know they've been wanting to move to @web/test-runner for a while. This is huge @joryphillips !

I think after we update the build system to Rollup we can remove the .js from imports in those test files. I needed to add those extensions for make to work correctly.

Not a hill to die on, but I think we should keep them anyways and not rely on node_modules resolution.

@matthewp matthewp merged commit a1ef484 into matthewp:main Oct 27, 2021
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