[Resolver] Snapshot and screenshot tests #72791
Conversation
| ); | ||
| }); | ||
| it('should render 1 resolver node', () => { | ||
| expect(reactRenderResult.queryAllByTestId('resolverNode')).toMatchInlineSnapshot(` |
There was a problem hiding this comment.
Nice, but this is gonna fail every time we visually change anything even though nothing functionally may have actually changed in the resolver
There was a problem hiding this comment.
no more functionality or visual changes allowed
There was a problem hiding this comment.
Cool, looks like my work is done for the day. 👍
| const coreStart: CoreStart = coreMock.createStart(); | ||
|
|
||
| const history = createMemoryHistory(); | ||
| reactRenderResult = render( |
There was a problem hiding this comment.
Wonder if we should put all our resolver tests in this file, or break them apart. I can see this file becoming massive as functionality of resolver expands... Thoughts?
There was a problem hiding this comment.
this is the only test.
There was a problem hiding this comment.
no more tests allowed
| const coreStart: CoreStart = coreMock.createStart(); | ||
|
|
||
| const history = createMemoryHistory(); | ||
| reactRenderResult = render( |
| | null; | ||
| } = React.createRef(); | ||
| const { colorMap, cubeAssetsForNode } = useResolverTheme(); | ||
| const { colorMap, cubeAssetsForNode } = useResolverTheme(resolverComponentInstanceID); |
There was a problem hiding this comment.
❔ I may have missed this, but why are we passing in a component instance ID to the theming hook? You won't have different themes for different Resolvers (AFAIK), right? But this makes it look like you could.
There was a problem hiding this comment.
just to have stable (non-random) html IDs for symbols
| <g> | ||
| <use | ||
| xlinkHref={`#${SymbolIds.processCubeActiveBacking}`} | ||
| xlinkHref={`#${symbolIDs(resolverComponentInstanceID).processCubeActiveBacking}`} |
There was a problem hiding this comment.
❔ Something isn't right here, this more or less defeats the purpose of using <symbol>s
| * A hook to bring Resolver theming information into components. | ||
| */ | ||
| export const useResolverTheme = (): { | ||
| export const useResolverTheme = ( |
There was a problem hiding this comment.
❔ re my comment in chat, we could memoize this right?
| const resolverComponentInstanceID = useSelector(selectors.resolverComponentInstanceID); | ||
| return ( | ||
| <svg className={className}> | ||
| <defs> |
There was a problem hiding this comment.
❔ this reflects "1 defs for each resolver". I think we should try instead for "1 defs for all Resolvers".
There was a problem hiding this comment.
but this is more correct than what we're doing now (1 def per resolver with duplicate IDs) so we should merge this 1st.
💔 Build Failed
Failed CI StepsBuild metricsasync chunks size
History
To update your PR or re-run it, just comment with: |
4 participants
Summary
This can 'render' resolver via
@testing-library/react. next step is to render it in the functional test runner and take a screenshot.Checklist
Delete any items that are not applicable to this PR.
For maintainers