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

feat(SandpackTests): Add SandpackTests component #562

Merged
merged 52 commits into from
Aug 31, 2022

Conversation

mattphillips
Copy link
Contributor

@mattphillips mattphillips commented Aug 25, 2022

CodeSandbox logoCodeSandbox logo  Open in CodeSandbox Web Editor | VS Code

What kind of change does this pull request introduce?

  • This PR adds a new SandpackTests component.
  • I've added the missing test related types to the sandpack-client project's SandpcakMessage type. Type defs have been adapted from: codeandbox tests component
  • I've also bumped package-build-stats dependency to a newer version. I was unable to build the project because of a node-sass issue related to the older version of this package (see related)

What is the current behavior?

This feature does not exist in Sandpack but builds on top of codesandbox's tests panel.

It is also the result of this Tweet

Closes #112

What is the new behavior?

The component handles:

  • Running multiple files tests
  • Running individual test files
  • Displaying test results
    • Including in a verbose mode
  • Handles file errors
  • Handles no tests file
  • Formats error messages
  • Should work with all themes

What steps did you take to test this? This is required before we can merge, make sure to test the flow you've updated.

  1. Manually tested
  2. Added a story in storybook for each of the above features

Checklist

  • Documentation;
  • Storybook (if applicable);
  • Tests;
  • Ready to be merged;

Todo before merging

  • Update the docs
  • Go over css to make sure it follows project style

Future Todos

I'll get to these once this PR is ready to merge.

  • Add some nicer styling to the verbose toggle. (Am still new to stitches and it doesn't feel that intuitive 😅).
  • jest-lite in codesandbox doesn't support jsx files but should so a PR is needed to change this line
  • jest-lite in codesandbox doesn't correctly handle focussed tests (.only). It treats focussed test as the only test for all files when it should be scoped to a describe level.
  • Expose an onComplete callback to SandpackTests for consumers to listen for results
  • Expose additional props that confirm to this projects API style for components e.g. style/className to customise the root SandpackStack.

Overall this is pretty much done but I still have a few bits to do. I think now would be a good time to get some eyes on this for some feedback.

cc/ @CompuIves @danilowoz PR as promised 😃

@codesandbox-ci
Copy link

codesandbox-ci bot commented Aug 25, 2022

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit feaf291:

Sandbox Source
Sandpack Configuration
jest-extended-sandbox (forked) Issue #112

@nicobytes
Copy link

Wow, great! I hope this component be accepted soon!

I'm working in a component for tests, but this proposal is so completed

image

@mattphillips
Copy link
Contributor Author

Hey @danilowoz, I've just been thinking about writing some docs for this PR and it got me wondering if we would want a tests template too?

I've had a quick look into how the templates work and it seems they only show the activeFile as being visible by default and only render the Preview component by default. To make a tests template we'd need to change some of this behaviour to allow templates to specify which component to render (SandpackPreview / SandpackTests).

An extension on this is that we could also add an example test to each template and an option whether to display the Tests panel as part of the Sandpack options when using templates (this would probably default to false).

Curious to hear your thoughts? 😄

@danilowoz
Copy link
Member

danilowoz commented Aug 26, 2022

@mattphillips loved the idea! Let's do it!

So, here's a quick summary of what's missing:

  • Add a new template: render the test component instead of preview;
  • Add watch mode: introduce a new toggle on the top bar;
  • Documentation and examples;
  • Make sure all colors: check with designers what's the to do here;

@mattphillips
Copy link
Contributor Author

Yeah that summary looks good to me 👍

One other thing we could do (could be in another PR) is to add a test example to each of the other templates

@danilowoz
Copy link
Member

Changelog

  • Add new SandpackTest component: watch and verbose options;
  • Introduce a new hook: useSandpackClient, which is responsible for registering a new client and returning its instance;
  • New template file: tests-ts;
  • Sandpack component introduces a new layout view: test mode;

Copy link
Member

@danilowoz danilowoz left a comment

Choose a reason for hiding this comment

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

🚀 🚀 🚀 🚀 🚀 🚀

@danilowoz danilowoz merged commit 1191f82 into codesandbox:main Aug 31, 2022
@danilowoz danilowoz deleted the mp/sandpack-test branch August 31, 2022 10:21
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.

Feature request: Add tests panel support
3 participants