Repository navigation
Task/setup storybook test framework - #294
MikeNeilson wants to merge 3 commits into
Conversation
|
Please set a versioning label of either |
|
Okay, I was having that same npm ci issue in act before I opened the PR. The thing does work, just the action being wonky. Though definitely possible it's an issue with the node version or something when package.lock got updated. Will investigate at some point. |
8141903 to
12146e0
Compare
|
Okay, apparently a rather hard reset of package-lock.json is required when that start happening. |
willbreitkreutz
left a comment
willbreitkreutz
left a comment
There was a problem hiding this comment.
Couple small changes, would suggest a rebase to main to pick up the updates that have been made, likely deleting the package-lock file and re-generating it after package.json is up to date should work.
| const copiedText = mockClipboard.readText(); | ||
| expect(copiedText).toBe(args.text); | ||
| // verify green class is removed. | ||
| waitFor(() => |
There was a problem hiding this comment.
Believe we need to await the waitFor call
| @@ -0,0 +1,14 @@ | |||
| import type { Preview } from '@storybook/react-vite' | |||
|
|
|||
There was a problem hiding this comment.
I think we need to import the css here to render stuff right
willbreitkreutz
left a comment
willbreitkreutz
left a comment
There was a problem hiding this comment.
Couple small changes, would suggest a rebase to main to pick up the updates that have been made, likely deleting the package-lock file and re-generating it after package.json is up to date should work.
This branch has not been deployed
2 participants
Summary
CodeBlock: This is a simple default render so a developer can see the component. I recommend every component have at least one of these static examples. E.g. no assertions. It is also useful to provide a Story that provides behavior mocks but no assertions. This allows exploratory operations. If there are assertions in the play method Storybook will reset it in the middle of you trying to do something.
CopyButton: Asserts the behavior of the copy button given some fixed text, As well as providing the default do nothing (except visual changes) Story.
Story had to mock navigator.clipboard has that component is striped out for "security reasons" when the tests are running in the browser and just not properly available on the terminal.
See https://storybook.js for more information.
Known gotcha that was not address in this initial PR. Any component that uses state or has callbacks and needs to have it behave correctly will likely need a "wrapper" render method (goes in the story) so that storybook can render that wrapper the way it needs. Sample is available here: https://github.com/opendcs/opendcs/blob/67d6d9e655132732796b006e94dc11fb35fe5dd9/javascript/opendcs-web-ui-react/src/pages/decodes/configs/Sample/SampleRetrieve.stories.tsx#L8
(Short version, if you keep having the thing pass in the browser but fail in odd ways on the console, that's probably what's going on.)
Additional tests should most definitely be added, I will do so as time permits and specific needs arise. I will also, of course, provide guidance should anyone start having issues with how things behave.
Otherwise consider implementing code coverage and some checks to make sure coverage doesn't go down when people submit new code. OpenDCS uses SonarCloud for that, there are other options. NOTE: getting the coverage to work with vitest + storybook will be more effort, especially here as the existing vitest config is separate from the new one. Consider combining that configuration into a single vitest.config.(ts|js) file as vitest supports "projects" (see https://github.com/opendcs/opendcs/blob/67d6d9e655132732796b006e94dc11fb35fe5dd9/javascript/opendcs-web-ui-react/vite.config.ts#L89).
Checklist
patch-bump,minor-bump, ormajor-bumpRelease impact