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

Component test: Fix wording #29487

Open
wants to merge 2 commits into
base: next
Choose a base branch
from
Open

Conversation

JReinhold
Copy link
Contributor

@JReinhold JReinhold commented Oct 30, 2024

Closes #

What I did

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

  • stories
  • unit tests
  • integration tests
  • end-to-end tests

Manual testing

This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!

Documentation

  • Add or update documentation reflecting your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Checklist for Maintainers

  • When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli/src/sandbox-templates.ts

  • Make sure this PR contains one of the labels below:

    Available labels
    • bug: Internal changes that fixes incorrect behavior.
    • maintenance: User-facing maintenance tasks.
    • dependencies: Upgrading (sometimes downgrading) dependencies.
    • build: Internal-facing build tooling & test updates. Will not show up in release changelog.
    • cleanup: Minor cleanup style change. Will not show up in release changelog.
    • documentation: Documentation only changes. Will not show up in release changelog.
    • feature request: Introducing a new feature.
    • BREAKING CHANGE: Changes that break compatibility in some way with current major version.
    • other: Changes that don't fit in the above categories.

🦋 Canary release

This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the @storybookjs/core team here.

core team members can create a canary release here or locally with gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>

name before after diff z %
createSize 0 B 0 B 0 B - -
generateSize 78.2 MB 78.2 MB 0 B 1.13 0%
initSize 143 MB 143 MB -16.5 kB 0.19 0%
diffSize 65.2 MB 65.1 MB -16.5 kB -0.29 0%
buildSize 6.88 MB 6.87 MB -1.51 kB -28.98 0%
buildSbAddonsSize 1.51 MB 1.51 MB -266 B -39.1 0%
buildSbCommonSize 195 kB 195 kB 0 B - 0%
buildSbManagerSize 1.9 MB 1.9 MB -967 B -19.57 -0.1%
buildSbPreviewSize 271 kB 271 kB -2 B -Infinity 0%
buildStaticSize 0 B 0 B 0 B - -
buildPrebuildSize 3.88 MB 3.88 MB -1.24 kB -23.49 0%
buildPreviewSize 3 MB 3 MB -271 B -Infinity 0%
testBuildSize 0 B 0 B 0 B - -
testBuildSbAddonsSize 0 B 0 B 0 B - -
testBuildSbCommonSize 0 B 0 B 0 B - -
testBuildSbManagerSize 0 B 0 B 0 B - -
testBuildSbPreviewSize 0 B 0 B 0 B - -
testBuildStaticSize 0 B 0 B 0 B - -
testBuildPrebuildSize 0 B 0 B 0 B - -
testBuildPreviewSize 0 B 0 B 0 B - -
name before after diff z %
createTime 22.9s 17.4s -5s -486ms 0.45 -31.4%
generateTime 23s 20.9s -2s -121ms -0.34 -10.1%
initTime 15.5s 15.6s 142ms 0.2 0.9%
buildTime 8.3s 7.6s -727ms -1.22 -9.5%
testBuildTime 0ms 0ms 0ms - -
devPreviewResponsive 5.4s 5.8s 389ms -0.16 6.6%
devManagerResponsive 3.4s 3.4s 89ms -0.5 2.5%
devManagerHeaderVisible 519ms 623ms 104ms 0.81 16.7%
devManagerIndexVisible 594ms 695ms 101ms 0.78 14.5%
devStoryVisibleUncached 862ms 1.1s 319ms 0.39 27%
devStoryVisible 592ms 693ms 101ms 0.98 14.6%
devAutodocsVisible 507ms 517ms 10ms -0.15 1.9%
devMDXVisible 520ms 584ms 64ms 0.67 11%
buildManagerHeaderVisible 490ms 580ms 90ms 0.09 15.5%
buildManagerIndexVisible 502ms 598ms 96ms 0.09 16.1%
buildStoryVisible 492ms 573ms 81ms 0.02 14.1%
buildAutodocsVisible 467ms 517ms 50ms 0.89 9.7%
buildMDXVisible 406ms 455ms 49ms -0.06 10.8%

Greptile Summary

Based on the provided information, I'll create a concise summary of the pull request changes:

Updates terminology from 'interaction testing' to 'component testing' across Storybook's codebase for better consistency and clarity.

  • Updated EmptyState components in interactions and test addons to use 'Component testing' title and description
  • Changed documentation links from 'writing-tests/interaction-testing' to 'writing-tests/component-testing' across multiple files
  • Updated comments and references in template files for all frameworks (React, Vue, Svelte, etc.) to use consistent terminology
  • Simplified UI by removing video tutorial links and dividers from EmptyState components
  • Updated error messages and documentation paths in core files to reflect new terminology

Copy link
Member

@ghengeveld ghengeveld left a comment

Choose a reason for hiding this comment

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

Just wondering whether the old interactions addons should be updated as well. I would expect that to just remain the same.

@@ -46,19 +40,15 @@ export const Empty = () => {

return (
<EmptyTabContent
title="Interaction testing"
Copy link
Member

Choose a reason for hiding this comment

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

Should this file be updated at all? I would expect the new terminology to only be used in the test addon, not the (legacy) interactions addon.

Copy link
Contributor

Choose a reason for hiding this comment

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

We've changed the term everywhere, including the Interactions addon (except that name itself 🙃).

Copy link

nx-cloud bot commented Oct 30, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 9fe5916. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

27 file(s) reviewed, 7 comment(s)
Edit PR Review Bot Settings | Greptile

@@ -126,7 +126,7 @@ export const babel = async (_: unknown, options: Options) => {
...babelDefault,
// This override makes sure that we will never transpile babel further down then the browsers that storybook supports.
Copy link
Contributor

Choose a reason for hiding this comment

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

syntax: typo: 'down then' should be 'down than'

@@ -228,7 +228,7 @@ export class MountMustBeDestructuredError extends StorybookError {

Note that Angular is not supported. As async/await is transpiled to support the zone.js polyfill.
Copy link
Contributor

Choose a reason for hiding this comment

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

syntax: sentence fragment: 'As async/await is transpiled' is incomplete and unclear

@@ -21,7 +21,7 @@ const meta: Meta<typeof MyPage> = {
export default meta;
type Story = StoryObj<typeof MyPage>;

// More on interaction testing: https://storybook.js.org/docs/writing-tests/interaction-testing
// More on component testing: https://storybook.js.org/docs/writing-tests/component-testing
export const LoggedIn: Story = {
play: async ({ canvasElement }: any) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

style: canvasElement should be properly typed rather than using 'any'

@@ -21,7 +21,7 @@ const meta = {
export default meta;
type Story = StoryObj<typeof meta>;

// More on interaction testing: https://storybook.js.org/docs/writing-tests/interaction-testing
// More on component testing: https://storybook.js.org/docs/writing-tests/component-testing
export const LoggedIn: Story = {
play: async ({ canvasElement }: any) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Type 'any' used for canvasElement - consider using a more specific type

Copy link
Contributor

@jonniebigodes jonniebigodes left a comment

Choose a reason for hiding this comment

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

@JReinhold thanks for taking the time to put together this pull request and adjust the documentation. Appreciate it 🙏 ! Left one item for you to look into when you have a moment.

I've already approved this on my end to unblock you

@@ -49,7 +49,7 @@ The test itself is defined inside a `play` function connected to a story. Here's

{/* prettier-ignore-start */}

<CodeSnippets path="login-form-with-play-function.md" usesCsf3 csf2Path="writing-tests/interaction-testing#snippet-login-form-with-play-function" />
<CodeSnippets path="login-form-with-play-function.md" usesCsf3 csf2Path="writing-tests/component-testing#snippet-login-form-with-play-function" />
Copy link
Contributor

Choose a reason for hiding this comment

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

@JReinhold if you're ok with it, you can remove the usesCSF3 and csf2Path props from the code snippets as they are no longer relevant.

Copy link
Contributor

@kylegach kylegach left a comment

Choose a reason for hiding this comment

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

Made a couple of wording updates to the empty states, but this otherwise looks great!

code/addons/interactions/src/components/EmptyState.tsx Outdated Show resolved Hide resolved
code/addons/test/src/components/EmptyState.tsx Outdated Show resolved Hide resolved
@@ -46,19 +40,15 @@ export const Empty = () => {

return (
<EmptyTabContent
title="Interaction testing"
Copy link
Contributor

Choose a reason for hiding this comment

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

We've changed the term everywhere, including the Interactions addon (except that name itself 🙃).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants