test(gazzodown): Cover with snapshot testing#39397
Open
test(gazzodown): Cover with snapshot testing#39397
Conversation
Contributor
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds many unit and snapshot tests across the gazzodown package and introduces a new timestamp subsystem (Time, Short/Long/Full/Relative formats, Timestamp dispatcher and wrapper) with refresh logic and an ErrorBoundary-based wrapper. Changes
Sequence Diagram(s)sequenceDiagram
participant Component as RelativeTime Component
participant Context as MarkupInteractionContext
participant Formatter as intlFormatDistance
participant Effect as useEffect
participant Timer as setInterval
Component->>Context: read locale/language
Component->>Formatter: compute initial relative text (time, now, locale)
Formatter-->>Component: return text
Component->>Component: compute timeToRefresh
Component->>Effect: set up interval with timeToRefresh
Effect->>Timer: setInterval(refresh, timeToRefresh)
Timer->>Formatter: on tick, compute updated relative text
Formatter-->>Timer: return updated text
Timer->>Component: update displayed text and recalc timeToRefresh
Component->>Timer: clear/set interval with new timeToRefresh
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
coderabbitai
bot
added
the
type: feature
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
6 issues found across 101 files
Note: This PR contains a large number of files. cubic only reviews up to 75 files per PR, so some files may not have been reviewed.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/gazzodown/src/elements/Timestamp/RelativeTime.tsx">
<violation number="1" location="packages/gazzodown/src/elements/Timestamp/RelativeTime.tsx:8">
P1: Past timestamps are always refreshed every second because the refresh delta uses a signed difference.</violation>
<violation number="2" location="packages/gazzodown/src/elements/Timestamp/RelativeTime.tsx:42">
P2: Interval updates ignore context language by hardcoding `'en'`, causing incorrect localization after refresh.</violation>
</file>
<file name="packages/gazzodown/src/code/CodeBlock.spec.tsx">
<violation number="1" location="packages/gazzodown/src/code/CodeBlock.spec.tsx:79">
P2: This test mutates global `navigator.clipboard` without restoring it, which can leak state to other tests and cause order-dependent failures.</violation>
</file>
<file name="packages/gazzodown/src/elements/Timestamp/Time.tsx">
<violation number="1" location="packages/gazzodown/src/elements/Timestamp/Time.tsx:10">
P2: `toLocaleString()` is used without fixed locale/time zone, making rendered output non-deterministic across environments and causing flaky snapshots.</violation>
</file>
<file name="packages/gazzodown/src/elements/Timestamp/Time.spec.tsx">
<violation number="1" location="packages/gazzodown/src/elements/Timestamp/Time.spec.tsx:24">
P2: `toLocaleString()` is environment-dependent; this assertion can be flaky across locales/time zones. Use a fixed locale/timeZone (or an ISO string) to make the test deterministic.</violation>
</file>
<file name="packages/gazzodown/src/elements/Timestamp/Timestamp.spec.tsx">
<violation number="1" location="packages/gazzodown/src/elements/Timestamp/Timestamp.spec.tsx:78">
P2: This snapshot is environment-dependent because the rendered `time` includes `toLocaleString()` and date-fns formatting in the local timezone. The snapshot can change across locales/timezones, so prefer a deterministic assertion (e.g., compare against a computed string or assert only stable attributes) or set a fixed timezone/locale for the test.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
| it('copies code to clipboard on copy button click', async () => { | ||
| const writeText = jest.fn().mockResolvedValue(undefined); | ||
| Object.assign(navigator, { clipboard: { writeText } }); |
Contributor
There was a problem hiding this comment.
P2: This test mutates global navigator.clipboard without restoring it, which can leak state to other tests and cause order-dependent failures.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/gazzodown/src/code/CodeBlock.spec.tsx, line 79:
<comment>This test mutates global `navigator.clipboard` without restoring it, which can leak state to other tests and cause order-dependent failures.</comment>
<file context>
@@ -0,0 +1,119 @@
+
+ it('copies code to clipboard on copy button click', async () => {
+ const writeText = jest.fn().mockResolvedValue(undefined);
+ Object.assign(navigator, { clipboard: { writeText } });
+
+ renderCodeBlock({ lines: codeLines(['const a = 1;', 'const b = 2;']) });
</file context>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (23)
packages/gazzodown/src/code/PreviewCodeBlock.spec.tsx (1)
39-41: Scope the snapshot to stable output to reduce fragility.Snapshotting the whole container tends to fail on unrelated structural changes. Prefer snapshotting stable content (e.g.,
textContent) or using explicit assertions for markup-independent behavior.♻️ Suggested change
- it('matches snapshot', () => { - const { container } = render(<PreviewCodeBlock lines={codeLines(['const x = 42;'])} />); - expect(container).toMatchSnapshot(); - }); + it('matches snapshot', () => { + const { container } = render(<PreviewCodeBlock lines={codeLines(['const x = 42;'])} />); + expect(container.textContent).toMatchInlineSnapshot('"const x = 42;"'); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/code/PreviewCodeBlock.spec.tsx` around lines 39 - 41, The test currently snapshots the entire DOM container which is brittle; update the 'matches snapshot' test in PreviewCodeBlock.spec.tsx to snapshot only stable output (for example use container.textContent or a specific element queried from render, e.g., getByTestId or querySelector for the code block) instead of the whole container; change the expect call from expect(container).toMatchSnapshot() to expect(container.textContent).toMatchSnapshot() or expect(renderedElement.textContent).toMatchSnapshot(), keeping the same render call using PreviewCodeBlock and codeLines.packages/gazzodown/src/code/PreviewCodeElement.spec.tsx (2)
11-14: Strengthen the empty-input test to assert component output, not just render plumbing.On Line 13, asserting only
containerpresence can pass even when the component renders an unexpected structure. Please assert expected DOM semantics for the empty state (e.g., renderedcodenode with empty content).Proposed refinement
it('renders empty string without crashing', () => { const { container } = render(<PreviewCodeElement code='' />); - expect(container).toBeInTheDocument(); + const codeElement = container.querySelector('code'); + expect(codeElement).toBeInTheDocument(); + expect(codeElement).toHaveTextContent(''); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/code/PreviewCodeElement.spec.tsx` around lines 11 - 14, The test "renders empty string without crashing" only asserts the container exists; update it to assert the actual rendered output for PreviewCodeElement when code='' by checking for a <code> node (or the appropriate role) and that its textContent is an empty string; modify the test that renders <PreviewCodeElement code='' /> (in PreviewCodeElement.spec.tsx) to query the rendered DOM for the code element (e.g., getByRole or container.querySelector('code')) and replace the current expect(container).toBeInTheDocument() with an assertion that the code element exists and has empty content.
16-19: Narrow snapshot scope to reduce fragility.On Line 18, snapshotting the full
containeris broad and can fail on unrelated wrapper/markup changes. Snapshotting the relevant node (or combining targeted assertions with a smaller snapshot) will make this suite more resilient.Proposed refinement
it('matches snapshot', () => { const { container } = render(<PreviewCodeElement code='const x = 42;' />); - expect(container).toMatchSnapshot(); + const codeElement = container.querySelector('code'); + expect(codeElement).toHaveTextContent('const x = 42;'); + expect(codeElement).toMatchSnapshot(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/code/PreviewCodeElement.spec.tsx` around lines 16 - 19, The test 'matches snapshot' is snapshotting the entire render container which is brittle; change it to snapshot only the relevant node from the render of PreviewCodeElement (e.g., use container.firstChild or a queried element like getByTestId/getByText for the code block) so the expectation targets the component output rather than surrounding wrappers — update the test in PreviewCodeElement.spec.tsx to obtain that specific DOM node (instead of using container) and call toMatchSnapshot on that node.packages/gazzodown/src/code/CodeBlock.spec.tsx (1)
77-87: Consider restoring the original clipboard after the test.Directly mutating
navigator.clipboardwithObject.assigncan leak state to subsequent tests if they rely on the original clipboard behavior. Restoring after the test improves isolation.♻️ Suggested fix with cleanup
it('copies code to clipboard on copy button click', async () => { const writeText = jest.fn().mockResolvedValue(undefined); + const originalClipboard = navigator.clipboard; Object.assign(navigator, { clipboard: { writeText } }); renderCodeBlock({ lines: codeLines(['const a = 1;', 'const b = 2;']) }); const button = await screen.findByTitle('Copy'); await userEvent.click(button); expect(writeText).toHaveBeenCalledWith('const a = 1;\nconst b = 2;'); + Object.assign(navigator, { clipboard: originalClipboard }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/code/CodeBlock.spec.tsx` around lines 77 - 87, The test "copies code to clipboard on copy button click" mutates navigator.clipboard and doesn't restore it; save the original clipboard (e.g., const originalClipboard = navigator.clipboard) before Object.assign in that spec and restore it after the test (using finally in the test or an afterEach cleanup) so other tests aren't affected; ensure you reference the same writeText mock (created in the spec) and reassign navigator.clipboard = originalClipboard when cleaning up.packages/gazzodown/src/PreviewMarkup.spec.tsx (1)
185-202: Consider verifying second line is not rendered for consistency.The list and task tests verify that only the first item is rendered by asserting the second item is absent. For consistency and complete coverage of the preview behavior, consider adding an assertion that the second code line is not rendered.
♻️ Suggested improvement
expect(screen.getByText('const x = 1;')).toBeInTheDocument(); + expect(screen.queryByText('const y = 2;')).not.toBeInTheDocument(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/PreviewMarkup.spec.tsx` around lines 185 - 202, Add an assertion to the PreviewMarkup spec to ensure only the first code line is shown by checking that the second line ("const y = 2;") is not rendered; locate the test case for PreviewMarkup (the it('renders code block preview with first line') block) and append an assertion using screen.queryByText for "const y = 2;" and expect it to not be in the document so the test mirrors the list/task coverage for “only first item shown.”packages/gazzodown/src/blocks/QuoteBlock.spec.tsx (2)
9-13: Extract a typed paragraph fixture helper to remove duplication.The repeated inline paragraph payloads make the tests noisier and harder to evolve. A small helper improves readability and keeps fixtures consistent.
Suggested refactor
+const paragraph = (value: string) => ({ + type: 'PARAGRAPH' as const, + value: [{ type: 'PLAIN_TEXT' as const, value }], +}); + describe('QuoteBlock', () => { it('renders blockquote element', () => { render( <QuoteBlock> - {[ - { - type: 'PARAGRAPH', - value: [{ type: 'PLAIN_TEXT', value: 'Quoted text' }], - }, - ]} + {[paragraph('Quoted text')]} </QuoteBlock>, ); @@ it('renders multiple paragraphs', () => { render( <QuoteBlock> - {[ - { type: 'PARAGRAPH', value: [{ type: 'PLAIN_TEXT', value: 'First' }] }, - { type: 'PARAGRAPH', value: [{ type: 'PLAIN_TEXT', value: 'Second' }] }, - ]} + {[paragraph('First'), paragraph('Second')]} </QuoteBlock>, ); @@ it('matches snapshot', () => { const { container } = render( - <QuoteBlock>{[{ type: 'PARAGRAPH', value: [{ type: 'PLAIN_TEXT', value: 'Snapshot quote' }] }]}</QuoteBlock>, + <QuoteBlock>{[paragraph('Snapshot quote')]}</QuoteBlock>, ); expect(container.firstElementChild).toMatchSnapshot(); }); });Also applies to: 25-27, 36-36
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/blocks/QuoteBlock.spec.tsx` around lines 9 - 13, Tests in QuoteBlock.spec.tsx repeat inline paragraph payloads; extract a reusable typed fixture helper (e.g., makeParagraph or paragraphNode) to return the object { type: 'PARAGRAPH', value: [{ type: 'PLAIN_TEXT', value }] } and replace the duplicated inline literals at the locations around the array entries (the quoted paragraph fixtures used in the test blocks) with calls to that helper to reduce noise and ensure consistent typing.
34-39: Narrow snapshot scope to reduce fragility.Line 38 snapshots the full
container, which is more likely to fail on incidental wrapper/markup changes. Snapshotting the rendered quote node (or using an inline snapshot for only the relevant subtree) will be more stable.Suggested refactor
- it('matches snapshot', () => { - const { container } = render( - <QuoteBlock>{[{ type: 'PARAGRAPH', value: [{ type: 'PLAIN_TEXT', value: 'Snapshot quote' }] }]}</QuoteBlock>, - ); - expect(container).toMatchSnapshot(); - }); + it('matches snapshot', () => { + const { container } = render( + <QuoteBlock>{[{ type: 'PARAGRAPH', value: [{ type: 'PLAIN_TEXT', value: 'Snapshot quote' }] }]}</QuoteBlock>, + ); + expect(container.firstElementChild).toMatchSnapshot(); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/blocks/QuoteBlock.spec.tsx` around lines 34 - 39, The test currently snapshots the whole container which is fragile; update the test in QuoteBlock.spec.tsx to snapshot only the rendered quote element: after rendering via render(<QuoteBlock>...), locate the quote node (e.g., by querying for the blockquote element, a role, or the text "Snapshot quote" using the render helpers) and call toMatchSnapshot() on that node instead of on container so only the relevant subtree produced by QuoteBlock is snapshotted.packages/gazzodown/src/blocks/HeadingBlock.spec.tsx (2)
22-24: Make the snapshot less fragile and more specific.At Lines 22–24, snapshotting
containeris broader than needed and more sensitive to wrapper-level changes. Snapshot the rendered node and give the test a behavior-specific name.♻️ Proposed fix
-it('matches snapshot', () => { +it('renders level-2 heading snapshot', () => { const { container } = render(<HeadingBlock level={2}>{[{ type: 'PLAIN_TEXT', value: 'Snapshot heading' }]}</HeadingBlock>); - expect(container).toMatchSnapshot(); + expect(container.firstChild).toMatchSnapshot(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/blocks/HeadingBlock.spec.tsx` around lines 22 - 24, Rename the vague test and make the snapshot targeted and less fragile by snapshotting the HeadingBlock's rendered node instead of the full container: update the test name (e.g., "renders level 2 heading with text") and change the expectation in the HeadingBlock.spec.tsx test to snapshot the specific node returned by render (for example use container.firstChild or the element found via getByText('Snapshot heading')) rather than container, keeping the test focused on the HeadingBlock component output.
12-13: Tighten the parameterized test title formatting.At Line 12, the title has two
%iplaceholders but only one parameter is supplied, which weakens readability in test output.♻️ Proposed fix
-it.each([1, 2, 3, 4] as const)('renders h%i when level is %i', (level) => { +it.each([1, 2, 3, 4] as const)('renders h%i when level is provided', (level) => {Based on learnings: "Use descriptive test names that clearly communicate expected behavior in Playwright tests".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/blocks/HeadingBlock.spec.tsx` around lines 12 - 13, The test title in HeadingBlock.spec.tsx uses two %i placeholders but the it.each([...]) supplies a single parameter; update the parameterized test title in the it.each call (the string passed to it.each(...)(...)) to use only one placeholder—e.g. change to "renders h%i for level" or another single-%i descriptive name—so it matches the single (level) parameter used in the test and keeps the title output clear.packages/gazzodown/src/colors/PreviewColorElement.spec.tsx (1)
32-40: Snapshot scope is broad and may become brittleBoth snapshots target the full container, which can fail on unrelated markup churn. Consider snapshotting a narrower node/value to reduce fragility.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/colors/PreviewColorElement.spec.tsx` around lines 32 - 40, The snapshots in PreviewColorElement.spec.tsx are too broad (they snapshot the entire container), making tests brittle; update the two tests that render PreviewColorElement so they assert a narrower target instead of container — for example, select the specific element that represents the color preview (add/read a stable attribute like data-testid='preview-color' on the PreviewColorElement component or query the specific class/role it renders) and call expect(node).toMatchSnapshot() or assert its relevant text/style/value; modify the tests that use render(...) and container to instead queryByTestId/getByTestId (or container.querySelector for the specific element) and snapshot that node.packages/gazzodown/src/colors/ColorElement.spec.tsx (1)
14-16: Remove lint suppression and use a stable selector for the swatchLine 14 introduces an inline suppression and Line 15 relies on DOM traversal, which makes this test more fragile than necessary.
Proposed refactor
- // eslint-disable-next-line testing-library/no-node-access - const swatch = text.querySelector('span'); - expect(swatch).toHaveStyle({ backgroundColor: 'rgba(128, 64, 32, 50.19607843137255%)' }); + const swatch = screen.getByTestId('color-swatch'); + expect(swatch).toHaveStyle({ backgroundColor: 'rgba(128, 64, 32, 50.19607843137255%)' });As per coding guidelines: "Avoid code comments in the implementation".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/colors/ColorElement.spec.tsx` around lines 14 - 16, Remove the inline eslint suppression and stop using DOM traversal; instead select the swatch via a stable Testing Library query. Replace the querySelector usage on the "text" container with a Testing Library call (e.g., within(text).getByTestId('swatch') or getByRole/getByLabelText as appropriate) and update the ColorElement component to add a stable attribute (e.g., data-testid="swatch") if one doesn't exist; keep the assertion on backgroundColor unchanged. Ensure you remove the comment "eslint-disable-next-line testing-library/no-node-access" and reference the variables "text" and "swatch" in the updated test.packages/gazzodown/src/katex/KatexElement.spec.tsx (1)
34-36: Snapshot can be simplified into focused assertions.This snapshot is likely fragile and provides limited value beyond the explicit checks already present.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/katex/KatexElement.spec.tsx` around lines 34 - 36, The snapshot test in KatexElement.spec.tsx (the it('matches snapshot') test that renders <KatexElement code='\\alpha + \\beta' /> and asserts on container) is fragile; replace it with focused assertions that verify the rendered output structure and content instead of a full DOM snapshot — for example assert that the component renders a KaTeX element (query for a .katex or similar class), that it contains the expected symbols/text (e.g., 'α' and 'β' or the appropriate textContent), and that any important attributes or wrappers exist; update the test name accordingly and remove the container.toMatchSnapshot() usage.packages/gazzodown/src/katex/PreviewKatexBlock.spec.tsx (1)
16-18: Snapshot can be replaced with targeted assertions for stability.Line 17-18 container snapshot is likely to break on incidental DOM changes unrelated to behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/katex/PreviewKatexBlock.spec.tsx` around lines 16 - 18, The snapshot test in PreviewKatexBlock.spec.tsx ("matches snapshot") is brittle; replace it with targeted assertions: render the PreviewKatexBlock via render(<PreviewKatexBlock code='\\frac{a}{b}' />) and assert on specific DOM/behavior from the component (e.g., that an element with the KaTeX output class exists, that it contains the expected formatted text or accessible name, and that no error message is present). Update the test to use queries like getByText/getByRole/getByTestId (or class-based checks against the rendered output) to verify the fraction is rendered rather than asserting the entire container snapshot.packages/gazzodown/src/katex/PreviewKatexElement.spec.tsx (1)
16-18: Prefer explicit structural assertions over broad snapshots here.Lines 16-18 snapshot the whole container, which is fragile to unrelated markup changes. A focused assertion on expected text/element shape will be more stable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/katex/PreviewKatexElement.spec.tsx` around lines 16 - 18, The snapshot test in PreviewKatexElement.spec.tsx is too broad; replace the container snapshot with focused structural assertions: render the PreviewKatexElement (render(<PreviewKatexElement code='\\int_0^1 f(x) dx' />)) and assert specific DOM outcomes such as the presence of the rendered math element (e.g., a .katex or math container from the component), that it contains the expected visible text or symbols (like the integral sign and "f(x)"), and any specific attributes/classes you rely on; update the test to use getByText/getByRole or querySelector against the component root instead of toMatchSnapshot() to make the test stable and targeted.packages/gazzodown/src/katex/KatexBlock.spec.tsx (1)
35-37: Consider replacing the broad snapshot with explicit expectations.The snapshot here is brittle and overlaps with behavior already asserted above.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/katex/KatexBlock.spec.tsx` around lines 35 - 37, The test "matches snapshot" in KatexBlock.spec.tsx uses a brittle snapshot for <KatexBlock code='\\sum_{i=0}^{n} i' />; replace it with explicit expectations that assert the rendered output and behavior instead of a snapshot: render the component (using render in the test), then assert specific DOM outcomes such as the presence of the KaTeX container element/class, that it contains the expected LaTeX-rendered text or accessible text (e.g., "∑" or the string "sum" / "i=0" depending on rendering), and any attributes or child elements you rely on; update the test name if needed and remove the expect(container).toMatchSnapshot() assertion so the test verifies concrete outcomes of KatexBlock rather than a snapshot.packages/gazzodown/src/blocks/TaskListBlock.spec.tsx (1)
23-32: Exercise task-check behavior through interaction, not only render-time invocation.The current assertion at Line 31 verifies only that
onTaskCheckedwas called during render. It does not prove the checkbox wiring invokes the returned handler when users interact.Proposed test hardening
-import { render, screen } from '@testing-library/react'; +import { fireEvent, render, screen } from '@testing-library/react'; @@ it('calls onTaskChecked from context when provided', () => { - const onTaskChecked = jest.fn(() => () => undefined); + const onTaskCheckedHandler = jest.fn(); + const onTaskChecked = jest.fn(() => onTaskCheckedHandler); render( <MarkupInteractionContext.Provider value={{ onTaskChecked }}> <TaskListBlock tasks={[{ type: 'TASK', status: false, value: [{ type: 'PLAIN_TEXT', value: 'Task' }] }]} /> </MarkupInteractionContext.Provider>, ); expect(onTaskChecked).toHaveBeenCalled(); + fireEvent.click(screen.getByRole('checkbox')); + expect(onTaskCheckedHandler).toHaveBeenCalledTimes(1); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/blocks/TaskListBlock.spec.tsx` around lines 23 - 32, The test in TaskListBlock.spec.tsx currently only checks that the context function onTaskChecked was invoked at render time; update the test to exercise the interactive path by providing onTaskChecked as a jest.fn that returns a second jest.fn (the actual handler), render TaskListBlock within MarkupInteractionContext.Provider, query the checkbox (e.g., getByRole('checkbox') or getByLabelText) and simulate a user click (using fireEvent.click or userEvent.click) on that checkbox, then assert that the inner handler returned by onTaskChecked was called and that onTaskChecked itself was invoked with the expected arguments; reference MarkupInteractionContext, TaskListBlock, and the onTaskChecked mock to locate and modify the test.packages/gazzodown/src/Markup.spec.tsx (1)
88-89: Container-wide snapshots on many already-asserted cases will likely make this suite brittle.Most of these tests already verify semantics via roles/text; full-container snapshots here are high-churn and will fail on incidental markup changes.
Also applies to: 105-106, 132-133, 159-160, 193-194, 224-225, 245-246, 267-268, 291-292, 307-308, 335-336, 363-364
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/Markup.spec.tsx` around lines 88 - 89, The suite uses broad container snapshots (expect(container).toMatchSnapshot()) which makes tests brittle; remove those container-wide snapshot assertions in Markup.spec.tsx and instead rely on the existing targeted assertions (role/text/attribute checks) already present in each test, or if snapshotting is required, snapshot only specific elements (e.g., snapshot the result of getByRole/getByText) so incidental markup changes don't break tests; update all instances of expect(container).toMatchSnapshot() in the file accordingly.packages/gazzodown/src/emoji/emoji.spec.tsx (1)
8-83: Extract a shared render helper to reduce repetitive provider setup.The
MarkupInteractionContext.Providerwrapper is repeated 8 times across the test suite (lines 8–83). Five of these tests use the identical default context value{ convertAsciiToEmoji: true, useEmoji: true }. A helper function would reduce boilerplate and make these tests easier to maintain when the context shape changes.Tests 3–5 use different context values and can pass them as a second parameter to the helper.
♻️ Suggested refactor
+const renderWithEmojiContext = ( + ui: React.ReactElement, + value: React.ContextType<typeof MarkupInteractionContext> = { convertAsciiToEmoji: true, useEmoji: true }, +) => render(<MarkupInteractionContext.Provider value={value}>{ui}</MarkupInteractionContext.Provider>); + it('renders unicode emoji via EmojiRenderer', () => { - render( - <MarkupInteractionContext.Provider value={{ convertAsciiToEmoji: true, useEmoji: true }}> - <Emoji type='EMOJI' value={undefined} unicode='🚀' /> - </MarkupInteractionContext.Provider>, - ); + renderWithEmojiContext(<Emoji type='EMOJI' value={undefined} unicode='🚀' />);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/emoji/emoji.spec.tsx` around lines 8 - 83, Multiple tests repeat wrapping Emoji with MarkupInteractionContext.Provider; extract a helper like renderWithContext that takes the element (e.g., <Emoji ... />) and an optional context override (defaulting to { convertAsciiToEmoji: true, useEmoji: true }) to render React Testing Library's render(...) once, then update all tests to call renderWithContext(...) instead of manually using MarkupInteractionContext.Provider; ensure the helper is used in tests that pass alternate contexts (the ones currently passing convertAsciiToEmoji/useEmoji false) and in the big/preview snapshot tests referencing Emoji and MarkupInteractionContext.Provider.packages/gazzodown/src/elements/InlineElements.spec.tsx (1)
60-84: Strengthen LINK/IMAGE tests with URL attribute assertions.These tests currently validate visible output, but not
href/srcintegrity.Suggested patch
it('renders LINK', () => { renderInlineElements([ { type: 'LINK', value: { src: { type: 'PLAIN_TEXT', value: 'https://example.com' }, label: { type: 'PLAIN_TEXT', value: 'Example' }, }, }, ]); - expect(screen.getByRole('link')).toHaveTextContent('Example'); + expect(screen.getByRole('link')).toHaveTextContent('Example'); + expect(screen.getByRole('link')).toHaveAttribute('href', 'https://example.com'); }); it('renders IMAGE', () => { renderInlineElements([ { type: 'IMAGE', value: { src: { type: 'PLAIN_TEXT', value: 'https://example.com/img.png' }, label: { type: 'PLAIN_TEXT', value: 'Alt text' }, }, }, ]); + expect(screen.getByRole('img')).toHaveAttribute('src', 'https://example.com/img.png'); expect(screen.getByRole('img')).toHaveAttribute('alt', 'Alt text'); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/elements/InlineElements.spec.tsx` around lines 60 - 84, Update the LINK and IMAGE tests in InlineElements.spec.tsx to also assert the underlying URL attributes: after rendering the LINK via renderInlineElements and locating it with screen.getByRole('link'), add an assertion that the element has href equal to 'https://example.com' (use toHaveAttribute('href', ...)); likewise after rendering the IMAGE and locating it with screen.getByRole('img'), assert the image has src equal to 'https://example.com/img.png' (use toHaveAttribute('src', ...)). This ensures renderInlineElements correctly sets the href/src values for the LINK and IMAGE cases.packages/gazzodown/src/elements/ImageElement.spec.tsx (1)
13-18: Assert link destination in the new-tab test.This currently validates
target/relonly; a wronghrefwould still pass.Suggested patch
it('opens image in new tab', () => { render(<ImageElement src='https://example.com/img.png' alt={{ type: 'PLAIN_TEXT', value: 'Photo' }} />); const link = screen.getByRole('link'); + expect(link).toHaveAttribute('href', 'https://example.com/img.png'); expect(link).toHaveAttribute('target', '_blank'); expect(link).toHaveAttribute('rel', 'noopener noreferrer'); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/elements/ImageElement.spec.tsx` around lines 13 - 18, The test "opens image in new tab" for ImageElement currently only checks target/rel; update the assertion to also verify the link's destination by asserting the anchor's href equals the provided src (the URL string passed to ImageElement). Locate the test that renders ImageElement and the link obtained via screen.getByRole('link') and add an expectation on that link's href (matching 'https://example.com/img.png') so the test fails if the wrong href is rendered.packages/gazzodown/src/elements/Timestamp/RelativeTime.tsx (1)
10-26: Remove inline implementation comments for this module.Line 10/15/20/25 comments are explanatory rather than essential and add noise in production code.
As per coding guidelines: "Avoid code comments in the implementation".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/elements/Timestamp/RelativeTime.tsx` around lines 10 - 26, Remove the inline explanatory comments inside the RelativeTime component/module: delete the four non-essential comments that precede the conditional return blocks (the ones referencing "less than 1 minute", "minutes range", "hours range", and "refresh the time in 1 hour") and leave only the code using the timeToRefresh variable and the existing return values so the logic remains unchanged but without implementation comments.packages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsx (1)
15-17: Prefer scoped queries over globaldocument.querySelector.At Line 16/32/48/64/80/96, global querying can couple tests to DOM state outside the render under test. Use
container.querySelectororscreen-based queries per test.Also applies to: 31-33, 47-49, 63-65, 79-81, 95-97
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsx` around lines 15 - 17, Replace the global document.querySelector usages in the DateTimeFormats tests with scoped queries so tests only inspect the rendered output: locate the assertions that call document.querySelector('time') (appearing in the DateTimeFormats.spec.tsx tests) and change them to use the render result's container.querySelector('time') or a testing-library screen/within query for that render, then keep the same assertion (e.g., expect(timeEl).toHaveAttribute('dateTime', date.toISOString())); apply this replacement for each occurrence reported (lines around the multiple tests).packages/gazzodown/src/elements/Timestamp/Timestamp.tsx (1)
16-31: Remove inline switch-case comments from implementation.These comments are redundant with the case literals and add maintenance noise.
As per coding guidelines, "Avoid code comments in the implementation".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/elements/Timestamp/Timestamp.tsx` around lines 16 - 31, The switch-case in the Timestamp component contains inline comments next to each case (e.g., in the switch handling 't', 'T', 'd', 'D', 'f', 'F', 'R') which are redundant; remove those inline comments while keeping the existing case labels and return expressions (ShortTime, LongTime, ShortDate, LongDate, FullDate, FullDateLong, RelativeTime) intact so the behavior is unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/gazzodown/src/blocks/SpoilerBlock.spec.tsx`:
- Around line 30-36: The test "reveals content on Space key" in
SpoilerBlock.spec.tsx only asserts that the button is removed and can
false-pass; update the test for the SpoilerBlock component (test case 'reveals
content on Space key') to also assert the revealed content appears after
pressing Space by checking for the rendered text "Secret" (use screen.getByText
or expect(...).toBeInTheDocument()) in addition to the existing button
disappearance assertion so the test verifies both UI changes.
In `@packages/gazzodown/src/colors/PreviewColorElement.spec.tsx`:
- Around line 17-20: The test "pads single-digit hex values with leading zero"
only renders zeros and won't catch padding bugs for non-zero single-digit
values; update the test in PreviewColorElement.spec.tsx to render
PreviewColorElement with non-zero single-digit channels (e.g., r={1}, g={15},
b={10}, a={255}) and assert the displayed hex includes the correctly padded hex
string (e.g., "010f0a") using screen.getByText(/010f0a/), ensuring
PreviewColorElement's hex formatting is validated for non-zero single-digit
cases.
In `@packages/gazzodown/src/elements/BoldSpan.spec.tsx`:
- Around line 29-56: The tests titled "renders nested strike inside bold" and
"renders inline code inside bold" don't assert that text is wrapped in a
<strong> element; update both tests that render the BoldSpan component to
explicitly assert the bold wrapper exists by locating the rendered text via
screen.getByText('Struck bold') and screen.getByText('code()') respectively and
then checking .closest('strong') (or expect(...).toBeInTheDocument() on that
element) so each test verifies the presence of the <strong> wrapper produced by
BoldSpan.
In `@packages/gazzodown/src/elements/SpoilerSpan.spec.tsx`:
- Around line 41-47: The test "reveals content on Space key" in
SpoilerSpan.spec.tsx only asserts the button is removed but doesn't assert the
spoiler text is shown; after calling userEvent.keyboard(' ') (in the same test
block that renders <SpoilerSpan>{plainChild('Secret')}</SpoilerSpan> and focuses
the button) add an assertion that the revealed content "Secret" is
present/visible (e.g., use screen.getByText('Secret') or await
screen.findByText('Secret') and assert toBeInTheDocument() or toBeVisible()) so
the test verifies the actual content is rendered after activation.
In `@packages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsx`:
- Around line 20-23: The snapshot tests for ShortTime (and the other
DateTimeFormats specs) are unstable because locale/timezone-dependent formatted
output varies; replace the container snapshot assertions with stable checks:
assert the element's dateTime attribute (already covered) and assert the
rendered text content explicitly against an expected string derived from a known
locale/timezone, or alternatively pin the locale/timezone in the test setup
before comparing; update tests referencing ShortTime and the other instances in
this file to use getByText/getByRole to read text content and compare to a
deterministic expected value or configure a fixed locale/timezone prior to
rendering.
In `@packages/gazzodown/src/elements/Timestamp/RelativeTime.spec.tsx`:
- Around line 49-61: The test 'updates text on interval' for the RelativeTime
component only asserts truthiness; change it to capture container.textContent
into a new variable (e.g., updatedText) after advancing timers and assert that
updatedText !== initialText to verify the component actually updated; ensure the
timer advance is wrapped in React's act (or use await waitFor) so the
component's state updates run before reading container.textContent.
In `@packages/gazzodown/src/elements/Timestamp/RelativeTime.tsx`:
- Around line 37-47: The interval callback in the RelativeTime component
regresses locale handling and misnames state: rename the state setter setTime to
setText for the text state (text/setText), extract const locale = language ??
'en' and use that instead of the hardcoded 'en' inside intlFormatDistance, use
the existing time variable (not value.getTime()) when computing the refreshed
text, keep setTimeToRefresh(getTimeToRefresh(time)) as-is, and update the
useEffect dependencies to include language (and/or locale) and time while
removing value so the effect re-runs when locale or time change.
- Around line 7-27: The getTimeToRefresh function currently uses time -
Date.now() which yields negative values for past timestamps and causes a 1s
refresh; change it to compute the absolute difference (e.g., const diff =
Math.abs(time - Date.now())) and use diff in the existing range checks so past
timestamps follow the same cadence as future ones while preserving the original
return values in getTimeToRefresh.
In `@packages/gazzodown/src/elements/Timestamp/Timestamp.spec.tsx`:
- Around line 56-67: The test "renders RelativeTime for format \"R\"" uses
jest.useFakeTimers() and sets system time but currently calls
jest.useRealTimers() only after assertions; wrap the fake-timer setup and the
render/assert sequence for the <Timestamp format='R' value={date} /> test inside
a try-finally so that jest.useRealTimers() is always invoked in the finally
block, ensuring cleanup even if the assertion against the <time> element's
dateTime attribute fails.
- Around line 76-79: The snapshot test for format "f" in Timestamp.spec.tsx is
locale-dependent; replace it with deterministic assertions like the other tests:
render(<Timestamp format='f' value={date} />) and assert on the element’s
attributes/text deterministically (e.g.
expect(element).toHaveAttribute('dateTime', date.toISOString()) or check
textContent against a fixed value derived from the known `date`), targeting the
rendered Timestamp element returned by render and the existing `date` variable
instead of using a snapshot.
In `@packages/gazzodown/src/elements/Timestamp/TimestampWrapper.spec.tsx`:
- Around line 43-45: The test's final assertion is too weak — in the
TimestampWrapper.spec.tsx test for TimestampWrapper/ErrorBoundary, replace
expect(container.textContent).toBeTruthy() with explicit checks that prove the
fallback rendered: assert that no <time> element exists (e.g.,
container.querySelector('time') is null) and assert the fallback text is present
(e.g., expect(container.textContent).toMatch(/Invalid Date|UTC|error/i) or the
exact UTC fallback string your component produces). This ensures the test
verifies absence of the normal <time> output and presence of the intended
fallback message.
- Around line 33-47: The test "renders fallback when child throws" sets a
console.error spy that may leak if the test throws; wrap the spy and test body
in a try-finally so jest.restoreAllMocks() always runs: create the spy via
jest.spyOn(console, 'error').mockImplementation(() => undefined) before the try,
execute render(<TimestampWrapper>{timestamp}</TimestampWrapper>) and the expect
inside try, and call jest.restoreAllMocks() inside finally; reference the test
block / spec name and the TimestampWrapper component to locate and update the
code.
In `@packages/gazzodown/src/emoji/EmojiRenderer.spec.tsx`:
- Around line 37-38: Add a call-count assertion to the EmojiRenderer test to
ensure detectEmoji is invoked exactly once: after mounting/rendering and before
asserting the DOM, add expect(detectEmoji).toHaveBeenCalledTimes(1) in the test
that currently calls expect(detectEmoji).toHaveBeenCalledWith(':smile:'); this
tightens the verification for the detectEmoji mock/function used in the test
(detectEmoji) and prevents extra unintended calls while keeping the existing
toHaveBeenCalledWith(':smile:') and the DOM assertion
(screen.getByTitle('smile')).
In `@packages/gazzodown/src/katex/PreviewKatexElement.spec.tsx`:
- Around line 11-14: Replace the plain text input with actual LaTeX command
tokens in the test for PreviewKatexElement: change the prop code='alpha + beta'
to use escaped backslashes like code='\\alpha + \\beta' (or a double-escaped
string if needed) and update the expectation to check for the literal command
text (expect(container).toHaveTextContent('\\alpha + \\beta')) so the test named
"renders LaTeX commands as plain text" actually validates rendering of LaTeX
commands; update the test case inside the it(...) block that calls
render(<PreviewKatexElement ... />) and its corresponding expect assertion.
In `@packages/gazzodown/src/Markup.spec.tsx`:
- Around line 285-288: The test mutates the live rendered DOM by calling
container.querySelector('math')?.remove(); instead clone the rendered output
before sanitizing: after awaiting waitFor(() =>
expect(container.querySelector('math')).toBeTruthy()) create a deep clone of
container (e.g., container.cloneNode(true)), remove the <math> node(s) from the
clone using querySelector/querySelectorAll on the clone, and then perform the
snapshot assertion (toMatchSnapshot) against the sanitized clone so the original
container remains untouched; update both occurrences around lines using
container.querySelector('math')?.remove() and any snapshot assertions that
follow.
---
Nitpick comments:
In `@packages/gazzodown/src/blocks/HeadingBlock.spec.tsx`:
- Around line 22-24: Rename the vague test and make the snapshot targeted and
less fragile by snapshotting the HeadingBlock's rendered node instead of the
full container: update the test name (e.g., "renders level 2 heading with text")
and change the expectation in the HeadingBlock.spec.tsx test to snapshot the
specific node returned by render (for example use container.firstChild or the
element found via getByText('Snapshot heading')) rather than container, keeping
the test focused on the HeadingBlock component output.
- Around line 12-13: The test title in HeadingBlock.spec.tsx uses two %i
placeholders but the it.each([...]) supplies a single parameter; update the
parameterized test title in the it.each call (the string passed to
it.each(...)(...)) to use only one placeholder—e.g. change to "renders h%i for
level" or another single-%i descriptive name—so it matches the single (level)
parameter used in the test and keeps the title output clear.
In `@packages/gazzodown/src/blocks/QuoteBlock.spec.tsx`:
- Around line 9-13: Tests in QuoteBlock.spec.tsx repeat inline paragraph
payloads; extract a reusable typed fixture helper (e.g., makeParagraph or
paragraphNode) to return the object { type: 'PARAGRAPH', value: [{ type:
'PLAIN_TEXT', value }] } and replace the duplicated inline literals at the
locations around the array entries (the quoted paragraph fixtures used in the
test blocks) with calls to that helper to reduce noise and ensure consistent
typing.
- Around line 34-39: The test currently snapshots the whole container which is
fragile; update the test in QuoteBlock.spec.tsx to snapshot only the rendered
quote element: after rendering via render(<QuoteBlock>...), locate the quote
node (e.g., by querying for the blockquote element, a role, or the text
"Snapshot quote" using the render helpers) and call toMatchSnapshot() on that
node instead of on container so only the relevant subtree produced by QuoteBlock
is snapshotted.
In `@packages/gazzodown/src/blocks/TaskListBlock.spec.tsx`:
- Around line 23-32: The test in TaskListBlock.spec.tsx currently only checks
that the context function onTaskChecked was invoked at render time; update the
test to exercise the interactive path by providing onTaskChecked as a jest.fn
that returns a second jest.fn (the actual handler), render TaskListBlock within
MarkupInteractionContext.Provider, query the checkbox (e.g.,
getByRole('checkbox') or getByLabelText) and simulate a user click (using
fireEvent.click or userEvent.click) on that checkbox, then assert that the inner
handler returned by onTaskChecked was called and that onTaskChecked itself was
invoked with the expected arguments; reference MarkupInteractionContext,
TaskListBlock, and the onTaskChecked mock to locate and modify the test.
In `@packages/gazzodown/src/code/CodeBlock.spec.tsx`:
- Around line 77-87: The test "copies code to clipboard on copy button click"
mutates navigator.clipboard and doesn't restore it; save the original clipboard
(e.g., const originalClipboard = navigator.clipboard) before Object.assign in
that spec and restore it after the test (using finally in the test or an
afterEach cleanup) so other tests aren't affected; ensure you reference the same
writeText mock (created in the spec) and reassign navigator.clipboard =
originalClipboard when cleaning up.
In `@packages/gazzodown/src/code/PreviewCodeBlock.spec.tsx`:
- Around line 39-41: The test currently snapshots the entire DOM container which
is brittle; update the 'matches snapshot' test in PreviewCodeBlock.spec.tsx to
snapshot only stable output (for example use container.textContent or a specific
element queried from render, e.g., getByTestId or querySelector for the code
block) instead of the whole container; change the expect call from
expect(container).toMatchSnapshot() to
expect(container.textContent).toMatchSnapshot() or
expect(renderedElement.textContent).toMatchSnapshot(), keeping the same render
call using PreviewCodeBlock and codeLines.
In `@packages/gazzodown/src/code/PreviewCodeElement.spec.tsx`:
- Around line 11-14: The test "renders empty string without crashing" only
asserts the container exists; update it to assert the actual rendered output for
PreviewCodeElement when code='' by checking for a <code> node (or the
appropriate role) and that its textContent is an empty string; modify the test
that renders <PreviewCodeElement code='' /> (in PreviewCodeElement.spec.tsx) to
query the rendered DOM for the code element (e.g., getByRole or
container.querySelector('code')) and replace the current
expect(container).toBeInTheDocument() with an assertion that the code element
exists and has empty content.
- Around line 16-19: The test 'matches snapshot' is snapshotting the entire
render container which is brittle; change it to snapshot only the relevant node
from the render of PreviewCodeElement (e.g., use container.firstChild or a
queried element like getByTestId/getByText for the code block) so the
expectation targets the component output rather than surrounding wrappers —
update the test in PreviewCodeElement.spec.tsx to obtain that specific DOM node
(instead of using container) and call toMatchSnapshot on that node.
In `@packages/gazzodown/src/colors/ColorElement.spec.tsx`:
- Around line 14-16: Remove the inline eslint suppression and stop using DOM
traversal; instead select the swatch via a stable Testing Library query. Replace
the querySelector usage on the "text" container with a Testing Library call
(e.g., within(text).getByTestId('swatch') or getByRole/getByLabelText as
appropriate) and update the ColorElement component to add a stable attribute
(e.g., data-testid="swatch") if one doesn't exist; keep the assertion on
backgroundColor unchanged. Ensure you remove the comment
"eslint-disable-next-line testing-library/no-node-access" and reference the
variables "text" and "swatch" in the updated test.
In `@packages/gazzodown/src/colors/PreviewColorElement.spec.tsx`:
- Around line 32-40: The snapshots in PreviewColorElement.spec.tsx are too broad
(they snapshot the entire container), making tests brittle; update the two tests
that render PreviewColorElement so they assert a narrower target instead of
container — for example, select the specific element that represents the color
preview (add/read a stable attribute like data-testid='preview-color' on the
PreviewColorElement component or query the specific class/role it renders) and
call expect(node).toMatchSnapshot() or assert its relevant text/style/value;
modify the tests that use render(...) and container to instead
queryByTestId/getByTestId (or container.querySelector for the specific element)
and snapshot that node.
In `@packages/gazzodown/src/elements/ImageElement.spec.tsx`:
- Around line 13-18: The test "opens image in new tab" for ImageElement
currently only checks target/rel; update the assertion to also verify the link's
destination by asserting the anchor's href equals the provided src (the URL
string passed to ImageElement). Locate the test that renders ImageElement and
the link obtained via screen.getByRole('link') and add an expectation on that
link's href (matching 'https://example.com/img.png') so the test fails if the
wrong href is rendered.
In `@packages/gazzodown/src/elements/InlineElements.spec.tsx`:
- Around line 60-84: Update the LINK and IMAGE tests in InlineElements.spec.tsx
to also assert the underlying URL attributes: after rendering the LINK via
renderInlineElements and locating it with screen.getByRole('link'), add an
assertion that the element has href equal to 'https://example.com' (use
toHaveAttribute('href', ...)); likewise after rendering the IMAGE and locating
it with screen.getByRole('img'), assert the image has src equal to
'https://example.com/img.png' (use toHaveAttribute('src', ...)). This ensures
renderInlineElements correctly sets the href/src values for the LINK and IMAGE
cases.
In `@packages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsx`:
- Around line 15-17: Replace the global document.querySelector usages in the
DateTimeFormats tests with scoped queries so tests only inspect the rendered
output: locate the assertions that call document.querySelector('time')
(appearing in the DateTimeFormats.spec.tsx tests) and change them to use the
render result's container.querySelector('time') or a testing-library
screen/within query for that render, then keep the same assertion (e.g.,
expect(timeEl).toHaveAttribute('dateTime', date.toISOString())); apply this
replacement for each occurrence reported (lines around the multiple tests).
In `@packages/gazzodown/src/elements/Timestamp/RelativeTime.tsx`:
- Around line 10-26: Remove the inline explanatory comments inside the
RelativeTime component/module: delete the four non-essential comments that
precede the conditional return blocks (the ones referencing "less than 1
minute", "minutes range", "hours range", and "refresh the time in 1 hour") and
leave only the code using the timeToRefresh variable and the existing return
values so the logic remains unchanged but without implementation comments.
In `@packages/gazzodown/src/elements/Timestamp/Timestamp.tsx`:
- Around line 16-31: The switch-case in the Timestamp component contains inline
comments next to each case (e.g., in the switch handling 't', 'T', 'd', 'D',
'f', 'F', 'R') which are redundant; remove those inline comments while keeping
the existing case labels and return expressions (ShortTime, LongTime, ShortDate,
LongDate, FullDate, FullDateLong, RelativeTime) intact so the behavior is
unchanged.
In `@packages/gazzodown/src/emoji/emoji.spec.tsx`:
- Around line 8-83: Multiple tests repeat wrapping Emoji with
MarkupInteractionContext.Provider; extract a helper like renderWithContext that
takes the element (e.g., <Emoji ... />) and an optional context override
(defaulting to { convertAsciiToEmoji: true, useEmoji: true }) to render React
Testing Library's render(...) once, then update all tests to call
renderWithContext(...) instead of manually using
MarkupInteractionContext.Provider; ensure the helper is used in tests that pass
alternate contexts (the ones currently passing convertAsciiToEmoji/useEmoji
false) and in the big/preview snapshot tests referencing Emoji and
MarkupInteractionContext.Provider.
In `@packages/gazzodown/src/katex/KatexBlock.spec.tsx`:
- Around line 35-37: The test "matches snapshot" in KatexBlock.spec.tsx uses a
brittle snapshot for <KatexBlock code='\\sum_{i=0}^{n} i' />; replace it with
explicit expectations that assert the rendered output and behavior instead of a
snapshot: render the component (using render in the test), then assert specific
DOM outcomes such as the presence of the KaTeX container element/class, that it
contains the expected LaTeX-rendered text or accessible text (e.g., "∑" or the
string "sum" / "i=0" depending on rendering), and any attributes or child
elements you rely on; update the test name if needed and remove the
expect(container).toMatchSnapshot() assertion so the test verifies concrete
outcomes of KatexBlock rather than a snapshot.
In `@packages/gazzodown/src/katex/KatexElement.spec.tsx`:
- Around line 34-36: The snapshot test in KatexElement.spec.tsx (the it('matches
snapshot') test that renders <KatexElement code='\\alpha + \\beta' /> and
asserts on container) is fragile; replace it with focused assertions that verify
the rendered output structure and content instead of a full DOM snapshot — for
example assert that the component renders a KaTeX element (query for a .katex or
similar class), that it contains the expected symbols/text (e.g., 'α' and 'β' or
the appropriate textContent), and that any important attributes or wrappers
exist; update the test name accordingly and remove the
container.toMatchSnapshot() usage.
In `@packages/gazzodown/src/katex/PreviewKatexBlock.spec.tsx`:
- Around line 16-18: The snapshot test in PreviewKatexBlock.spec.tsx ("matches
snapshot") is brittle; replace it with targeted assertions: render the
PreviewKatexBlock via render(<PreviewKatexBlock code='\\frac{a}{b}' />) and
assert on specific DOM/behavior from the component (e.g., that an element with
the KaTeX output class exists, that it contains the expected formatted text or
accessible name, and that no error message is present). Update the test to use
queries like getByText/getByRole/getByTestId (or class-based checks against the
rendered output) to verify the fraction is rendered rather than asserting the
entire container snapshot.
In `@packages/gazzodown/src/katex/PreviewKatexElement.spec.tsx`:
- Around line 16-18: The snapshot test in PreviewKatexElement.spec.tsx is too
broad; replace the container snapshot with focused structural assertions: render
the PreviewKatexElement (render(<PreviewKatexElement code='\\int_0^1 f(x) dx'
/>)) and assert specific DOM outcomes such as the presence of the rendered math
element (e.g., a .katex or math container from the component), that it contains
the expected visible text or symbols (like the integral sign and "f(x)"), and
any specific attributes/classes you rely on; update the test to use
getByText/getByRole or querySelector against the component root instead of
toMatchSnapshot() to make the test stable and targeted.
In `@packages/gazzodown/src/Markup.spec.tsx`:
- Around line 88-89: The suite uses broad container snapshots
(expect(container).toMatchSnapshot()) which makes tests brittle; remove those
container-wide snapshot assertions in Markup.spec.tsx and instead rely on the
existing targeted assertions (role/text/attribute checks) already present in
each test, or if snapshotting is required, snapshot only specific elements
(e.g., snapshot the result of getByRole/getByText) so incidental markup changes
don't break tests; update all instances of expect(container).toMatchSnapshot()
in the file accordingly.
In `@packages/gazzodown/src/PreviewMarkup.spec.tsx`:
- Around line 185-202: Add an assertion to the PreviewMarkup spec to ensure only
the first code line is shown by checking that the second line ("const y = 2;")
is not rendered; locate the test case for PreviewMarkup (the it('renders code
block preview with first line') block) and append an assertion using
screen.queryByText for "const y = 2;" and expect it to not be in the document so
the test mirrors the list/task coverage for “only first item shown.”
Comment on lines
+30
to
+36
| it('reveals content on Space key', async () => { | ||
| render(<SpoilerBlock>{[{ type: 'PARAGRAPH', value: [{ type: 'PLAIN_TEXT', value: 'Secret' }] }]}</SpoilerBlock>); | ||
|
|
||
| screen.getByRole('button').focus(); | ||
| await userEvent.keyboard(' '); | ||
| expect(screen.queryByRole('button')).not.toBeInTheDocument(); | ||
| }); |
Contributor
There was a problem hiding this comment.
Space-key test should assert the revealed content too.
Line 35 currently only checks button removal; adding a content assertion prevents false positives.
Suggested patch
it('reveals content on Space key', async () => {
render(<SpoilerBlock>{[{ type: 'PARAGRAPH', value: [{ type: 'PLAIN_TEXT', value: 'Secret' }] }]}</SpoilerBlock>);
screen.getByRole('button').focus();
await userEvent.keyboard(' ');
expect(screen.queryByRole('button')).not.toBeInTheDocument();
+ expect(screen.getByText('Secret')).toBeInTheDocument();
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| it('reveals content on Space key', async () => { | |
| render(<SpoilerBlock>{[{ type: 'PARAGRAPH', value: [{ type: 'PLAIN_TEXT', value: 'Secret' }] }]}</SpoilerBlock>); | |
| screen.getByRole('button').focus(); | |
| await userEvent.keyboard(' '); | |
| expect(screen.queryByRole('button')).not.toBeInTheDocument(); | |
| }); | |
| it('reveals content on Space key', async () => { | |
| render(<SpoilerBlock>{[{ type: 'PARAGRAPH', value: [{ type: 'PLAIN_TEXT', value: 'Secret' }] }]}</SpoilerBlock>); | |
| screen.getByRole('button').focus(); | |
| await userEvent.keyboard(' '); | |
| expect(screen.queryByRole('button')).not.toBeInTheDocument(); | |
| expect(screen.getByText('Secret')).toBeInTheDocument(); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/gazzodown/src/blocks/SpoilerBlock.spec.tsx` around lines 30 - 36,
The test "reveals content on Space key" in SpoilerBlock.spec.tsx only asserts
that the button is removed and can false-pass; update the test for the
SpoilerBlock component (test case 'reveals content on Space key') to also assert
the revealed content appears after pressing Space by checking for the rendered
text "Secret" (use screen.getByText or expect(...).toBeInTheDocument()) in
addition to the existing button disappearance assertion so the test verifies
both UI changes.
Comment on lines
+17
to
+20
| it('pads single-digit hex values with leading zero', () => { | ||
| render(<PreviewColorElement r={0} g={0} b={0} a={255} />); | ||
| expect(screen.getByText(/000000/)).toBeInTheDocument(); | ||
| }); |
Contributor
There was a problem hiding this comment.
Padding test doesn’t prove non-zero single-digit hex padding
Line 18 only tests zeros, so it won’t catch bugs for values like 1 → 01 or 15 → 0f.
Proposed test fix
- it('pads single-digit hex values with leading zero', () => {
- render(<PreviewColorElement r={0} g={0} b={0} a={255} />);
- expect(screen.getByText(/000000/)).toBeInTheDocument();
- });
+ it('pads single-digit hex values with leading zero', () => {
+ render(<PreviewColorElement r={1} g={10} b={15} a={255} />);
+ expect(screen.getByText(/010a0f/i)).toBeInTheDocument();
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| it('pads single-digit hex values with leading zero', () => { | |
| render(<PreviewColorElement r={0} g={0} b={0} a={255} />); | |
| expect(screen.getByText(/000000/)).toBeInTheDocument(); | |
| }); | |
| it('pads single-digit hex values with leading zero', () => { | |
| render(<PreviewColorElement r={1} g={10} b={15} a={255} />); | |
| expect(screen.getByText(/010a0f/i)).toBeInTheDocument(); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/gazzodown/src/colors/PreviewColorElement.spec.tsx` around lines 17 -
20, The test "pads single-digit hex values with leading zero" only renders zeros
and won't catch padding bugs for non-zero single-digit values; update the test
in PreviewColorElement.spec.tsx to render PreviewColorElement with non-zero
single-digit channels (e.g., r={1}, g={15}, b={10}, a={255}) and assert the
displayed hex includes the correctly padded hex string (e.g., "010f0a") using
screen.getByText(/010f0a/), ensuring PreviewColorElement's hex formatting is
validated for non-zero single-digit cases.
Comment on lines
+29
to
+56
| it('renders nested strike inside bold', () => { | ||
| render( | ||
| <BoldSpan> | ||
| {[ | ||
| { | ||
| type: 'STRIKE', | ||
| value: [{ type: 'PLAIN_TEXT', value: 'Struck bold' }], | ||
| }, | ||
| ]} | ||
| </BoldSpan>, | ||
| ); | ||
| // eslint-disable-next-line testing-library/no-node-access | ||
| expect(screen.getByText('Struck bold').closest('del')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('renders inline code inside bold', () => { | ||
| render( | ||
| <BoldSpan> | ||
| {[ | ||
| { | ||
| type: 'INLINE_CODE', | ||
| value: { type: 'PLAIN_TEXT', value: 'code()' }, | ||
| }, | ||
| ]} | ||
| </BoldSpan>, | ||
| ); | ||
| expect(screen.getByText('code()')).toBeInTheDocument(); | ||
| }); |
Contributor
There was a problem hiding this comment.
Two tests don’t verify the bold wrapper despite their titles.
renders nested strike inside bold and renders inline code inside bold can pass even if <strong> is missing.
Suggested patch
it('renders nested strike inside bold', () => {
render(
<BoldSpan>
{[
{
type: 'STRIKE',
value: [{ type: 'PLAIN_TEXT', value: 'Struck bold' }],
},
]}
</BoldSpan>,
);
// eslint-disable-next-line testing-library/no-node-access
expect(screen.getByText('Struck bold').closest('del')).toBeInTheDocument();
+ // eslint-disable-next-line testing-library/no-node-access
+ expect(screen.getByText('Struck bold').closest('strong')).toBeInTheDocument();
});
it('renders inline code inside bold', () => {
render(
<BoldSpan>
{[
{
type: 'INLINE_CODE',
value: { type: 'PLAIN_TEXT', value: 'code()' },
},
]}
</BoldSpan>,
);
expect(screen.getByText('code()')).toBeInTheDocument();
+ // eslint-disable-next-line testing-library/no-node-access
+ expect(screen.getByText('code()').closest('strong')).toBeInTheDocument();
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/gazzodown/src/elements/BoldSpan.spec.tsx` around lines 29 - 56, The
tests titled "renders nested strike inside bold" and "renders inline code inside
bold" don't assert that text is wrapped in a <strong> element; update both tests
that render the BoldSpan component to explicitly assert the bold wrapper exists
by locating the rendered text via screen.getByText('Struck bold') and
screen.getByText('code()') respectively and then checking .closest('strong') (or
expect(...).toBeInTheDocument() on that element) so each test verifies the
presence of the <strong> wrapper produced by BoldSpan.
Comment on lines
+41
to
+47
| it('reveals content on Space key', async () => { | ||
| render(<SpoilerSpan>{plainChild('Secret')}</SpoilerSpan>); | ||
|
|
||
| screen.getByRole('button').focus(); | ||
| await userEvent.keyboard(' '); | ||
| expect(screen.queryByRole('button')).not.toBeInTheDocument(); | ||
| }); |
Contributor
There was a problem hiding this comment.
Space-key reveal test is missing a content assertion.
The test should verify the spoiler text is actually rendered after key activation, not only that the button disappeared.
Suggested patch
it('reveals content on Space key', async () => {
render(<SpoilerSpan>{plainChild('Secret')}</SpoilerSpan>);
screen.getByRole('button').focus();
await userEvent.keyboard(' ');
expect(screen.queryByRole('button')).not.toBeInTheDocument();
+ expect(screen.getByText('Secret')).toBeInTheDocument();
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/gazzodown/src/elements/SpoilerSpan.spec.tsx` around lines 41 - 47,
The test "reveals content on Space key" in SpoilerSpan.spec.tsx only asserts the
button is removed but doesn't assert the spoiler text is shown; after calling
userEvent.keyboard(' ') (in the same test block that renders
<SpoilerSpan>{plainChild('Secret')}</SpoilerSpan> and focuses the button) add an
assertion that the revealed content "Secret" is present/visible (e.g., use
screen.getByText('Secret') or await screen.findByText('Secret') and assert
toBeInTheDocument() or toBeVisible()) so the test verifies the actual content is
rendered after activation.
Comment on lines
+33
to
+47
| it('renders fallback when child throws', () => { | ||
| jest.spyOn(console, 'error').mockImplementation(() => undefined); | ||
|
|
||
| const timestamp = { | ||
| type: 'TIMESTAMP' as const, | ||
| value: { timestamp: 'not-a-number', format: 'f' as const }, | ||
| }; | ||
|
|
||
| const { container } = render(<TimestampWrapper>{timestamp}</TimestampWrapper>); | ||
|
|
||
| // ErrorBoundary should render the fallback (UTC string of invalid date or error) | ||
| expect(container.textContent).toBeTruthy(); | ||
|
|
||
| jest.restoreAllMocks(); | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and view the test content
if [ -f "packages/gazzodown/src/elements/Timestamp/TimestampWrapper.spec.tsx" ]; then
echo "=== File exists, showing content ==="
cat -n "packages/gazzodown/src/elements/Timestamp/TimestampWrapper.spec.tsx"
else
echo "File not found at expected path"
fiRepository: RocketChat/Rocket.Chat
Length of output: 2383
🏁 Script executed:
# Search for Jest spy and mock restoration patterns in the timestamp test file
rg -n "spyOn|mockRestore|restoreAllMocks|afterEach|beforeEach" \
"packages/gazzodown/src/elements/Timestamp/" --type tsxRepository: RocketChat/Rocket.Chat
Length of output: 93
🏁 Script executed:
# Check if there are any jest.restoreAllMocks or afterEach hooks at the describe/test suite level
rg -B5 -A5 "renders fallback when child throws" \
"packages/gazzodown/src/elements/Timestamp/TimestampWrapper.spec.tsx"Repository: RocketChat/Rocket.Chat
Length of output: 485
Restore the console.error spy in a guaranteed cleanup path.
If the test fails before line 46 (e.g., at the render call or expect assertion), the spy will leak into subsequent tests since jest.restoreAllMocks() will never execute.
Use a try-finally block to ensure cleanup regardless of test outcome:
Proposed fix
it('renders fallback when child throws', () => {
- jest.spyOn(console, 'error').mockImplementation(() => undefined);
+ const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => undefined);
- const timestamp = {
- type: 'TIMESTAMP' as const,
- value: { timestamp: 'not-a-number', format: 'f' as const },
- };
-
- const { container } = render(<TimestampWrapper>{timestamp}</TimestampWrapper>);
-
- // ErrorBoundary should render the fallback (UTC string of invalid date or error)
- expect(container.textContent).toBeTruthy();
-
- jest.restoreAllMocks();
+ try {
+ const timestamp = {
+ type: 'TIMESTAMP' as const,
+ value: { timestamp: 'not-a-number', format: 'f' as const },
+ };
+ const { container } = render(<TimestampWrapper>{timestamp}</TimestampWrapper>);
+ expect(container.textContent).toBeTruthy();
+ } finally {
+ consoleErrorSpy.mockRestore();
+ }
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/gazzodown/src/elements/Timestamp/TimestampWrapper.spec.tsx` around
lines 33 - 47, The test "renders fallback when child throws" sets a
console.error spy that may leak if the test throws; wrap the spy and test body
in a try-finally so jest.restoreAllMocks() always runs: create the spy via
jest.spyOn(console, 'error').mockImplementation(() => undefined) before the try,
execute render(<TimestampWrapper>{timestamp}</TimestampWrapper>) and the expect
inside try, and call jest.restoreAllMocks() inside finally; reference the test
block / spec name and the TimestampWrapper component to locate and update the
code.
Comment on lines
+43
to
+45
| // ErrorBoundary should render the fallback (UTC string of invalid date or error) | ||
| expect(container.textContent).toBeTruthy(); | ||
|
|
Contributor
There was a problem hiding this comment.
Fallback assertion is too weak to prove fallback rendering.
expect(container.textContent).toBeTruthy() can pass for unrelated output. Assert the fallback signal more explicitly (e.g., absence of <time> and/or expected fallback text).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/gazzodown/src/elements/Timestamp/TimestampWrapper.spec.tsx` around
lines 43 - 45, The test's final assertion is too weak — in the
TimestampWrapper.spec.tsx test for TimestampWrapper/ErrorBoundary, replace
expect(container.textContent).toBeTruthy() with explicit checks that prove the
fallback rendered: assert that no <time> element exists (e.g.,
container.querySelector('time') is null) and assert the fallback text is present
(e.g., expect(container.textContent).toMatch(/Invalid Date|UTC|error/i) or the
exact UTC fallback string your component produces). This ensures the test
verifies absence of the normal <time> output and presence of the intended
fallback message.
Comment on lines
+37
to
+38
| expect(detectEmoji).toHaveBeenCalledWith(':smile:'); | ||
| expect(screen.getByTitle('smile')).toBeInTheDocument(); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/gazzodown/src/emoji/EmojiRenderer.spec.tsx | head -60Repository: RocketChat/Rocket.Chat
Length of output: 2080
Strengthen callback assertion with call count.
At Line 37, toHaveBeenCalledWith(':smile:') does not fail if extra unintended calls happen. Add a call-count check for tighter behavior guarantees.
Suggested tightening
- expect(detectEmoji).toHaveBeenCalledWith(':smile:');
+ expect(detectEmoji).toHaveBeenCalledTimes(1);
+ expect(detectEmoji).toHaveBeenCalledWith(':smile:');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| expect(detectEmoji).toHaveBeenCalledWith(':smile:'); | |
| expect(screen.getByTitle('smile')).toBeInTheDocument(); | |
| expect(detectEmoji).toHaveBeenCalledTimes(1); | |
| expect(detectEmoji).toHaveBeenCalledWith(':smile:'); | |
| expect(screen.getByTitle('smile')).toBeInTheDocument(); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/gazzodown/src/emoji/EmojiRenderer.spec.tsx` around lines 37 - 38,
Add a call-count assertion to the EmojiRenderer test to ensure detectEmoji is
invoked exactly once: after mounting/rendering and before asserting the DOM, add
expect(detectEmoji).toHaveBeenCalledTimes(1) in the test that currently calls
expect(detectEmoji).toHaveBeenCalledWith(':smile:'); this tightens the
verification for the detectEmoji mock/function used in the test (detectEmoji)
and prevents extra unintended calls while keeping the existing
toHaveBeenCalledWith(':smile:') and the DOM assertion
(screen.getByTitle('smile')).
Comment on lines
+11
to
+14
| it('renders LaTeX commands as plain text', () => { | ||
| const { container } = render(<PreviewKatexElement code='alpha + beta' />); | ||
| expect(container).toHaveTextContent('alpha + beta'); | ||
| }); |
Contributor
There was a problem hiding this comment.
Use real LaTeX command tokens in the command-focused case.
At Line 12, the input is plain text (alpha + beta), so this test does not validate command-like content despite the test name.
Proposed fix
- it('renders LaTeX commands as plain text', () => {
- const { container } = render(<PreviewKatexElement code='alpha + beta' />);
- expect(container).toHaveTextContent('alpha + beta');
+ it('renders LaTeX commands as plain text', () => {
+ const { container } = render(<PreviewKatexElement code='\\alpha + \\beta' />);
+ expect(container).toHaveTextContent('\\alpha + \\beta');
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| it('renders LaTeX commands as plain text', () => { | |
| const { container } = render(<PreviewKatexElement code='alpha + beta' />); | |
| expect(container).toHaveTextContent('alpha + beta'); | |
| }); | |
| it('renders LaTeX commands as plain text', () => { | |
| const { container } = render(<PreviewKatexElement code='\\alpha + \\beta' />); | |
| expect(container).toHaveTextContent('\\alpha + \\beta'); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/gazzodown/src/katex/PreviewKatexElement.spec.tsx` around lines 11 -
14, Replace the plain text input with actual LaTeX command tokens in the test
for PreviewKatexElement: change the prop code='alpha + beta' to use escaped
backslashes like code='\\alpha + \\beta' (or a double-escaped string if needed)
and update the expectation to check for the literal command text
(expect(container).toHaveTextContent('\\alpha + \\beta')) so the test named
"renders LaTeX commands as plain text" actually validates rendering of LaTeX
commands; update the test case inside the it(...) block that calls
render(<PreviewKatexElement ... />) and its corresponding expect assertion.
Comment on lines
+285
to
288
| // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access | ||
| await waitFor(() => expect(container.querySelector('math')).toBeTruthy()); | ||
| // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access | ||
| container.querySelector('math')?.remove(); |
Contributor
There was a problem hiding this comment.
Avoid mutating the live rendered DOM before snapshotting.
At Line 288, removing <math> from container changes the subject under test and can mask regressions. Snapshot a sanitized clone instead.
Safer snapshot approach without mutating rendered DOM
await waitFor(() => expect(container.querySelector('math')).toBeTruthy());
- container.querySelector('math')?.remove();
+ const snapshotContainer = container.cloneNode(true) as HTMLElement;
+ snapshotContainer.querySelector('math')?.remove();
@@
- expect(container).toMatchSnapshot();
+ expect(snapshotContainer).toMatchSnapshot();Also applies to: 291-291
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/gazzodown/src/Markup.spec.tsx` around lines 285 - 288, The test
mutates the live rendered DOM by calling
container.querySelector('math')?.remove(); instead clone the rendered output
before sanitizing: after awaiting waitFor(() =>
expect(container.querySelector('math')).toBeTruthy()) create a deep clone of
container (e.g., container.cloneNode(true)), remove the <math> node(s) from the
clone using querySelector/querySelectorAll on the clone, and then perform the
snapshot assertion (toMatchSnapshot) against the sanitized clone so the original
container remains untouched; update both occurrences around lines using
container.querySelector('math')?.remove() and any snapshot assertions that
follow.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #39397 +/- ##
===========================================
+ Coverage 70.95% 71.25% +0.29%
===========================================
Files 3197 3208 +11
Lines 113338 113452 +114
Branches 20567 20529 -38
===========================================
+ Hits 80423 80837 +414
+ Misses 30871 30569 -302
- Partials 2044 2046 +2
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
tassoevan
force-pushed
the
test/gazzodown-snapshots
branch
from
769f148 to
fc2e45c
Compare
coderabbitai
bot
added
type: chore
and removed
type: feature
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (1)
packages/gazzodown/src/emoji/EmojiRenderer.spec.tsx (1)
37-38:⚠️ Potential issue | 🟡 MinorTighten the callback assertion with a call count.
toHaveBeenCalledWith(':smile:')still passes ifdetectEmojiis invoked extra times. AddtoHaveBeenCalledTimes(1)before it so this test also catches duplicate evaluations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/emoji/EmojiRenderer.spec.tsx` around lines 37 - 38, Add a strict call-count assertion for the mock detectEmoji in EmojiRenderer.spec.tsx: before the existing expect(detectEmoji).toHaveBeenCalledWith(':smile:'), add expect(detectEmoji).toHaveBeenCalledTimes(1) so the test fails if detectEmoji is invoked more than once; locate the mock/spy named detectEmoji in the spec and insert the call count assertion immediately prior to the toHaveBeenCalledWith check.
🧹 Nitpick comments (7)
packages/gazzodown/src/elements/Timestamp/RelativeTime.tsx (1)
10-25: Remove inline code comments.The function contains explanatory comments on lines 10, 15-16, 20, and 25. As per coding guidelines, avoid code comments in the implementation for TypeScript files.
Suggested fix
const getTimeToRefresh = (time: number): number => { - const timeToRefresh = time - Date.now(); - - // less than 1 minute - if (timeToRefresh < 60000) { + const timeToRefresh = Math.abs(time - Date.now()); + + if (timeToRefresh < 60000) { return 1000; } - // if the difference is in the minutes range, we should refresh the time in 1 minute / 2 if (timeToRefresh < 3600000) { return 60000 / 2; } - // if the difference is in the hours range, we should refresh the time in 5 minutes if (timeToRefresh < 86400000) { return 60000 * 5; } - // refresh the time in 1 hour return 3600000; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/elements/Timestamp/RelativeTime.tsx` around lines 10 - 25, Remove the inline explanatory comments in RelativeTime.tsx that precede each if block (the comments describing "less than 1 minute", "minutes range", "hours range", and "refresh the time in 1 hour") while keeping the logic and return values intact; locate the conditional branch logic inside the RelativeTime component/function (the timeToRefresh checks and their returns 1000, 60000/2, 60000*5, etc.) and delete only the comment lines so the code remains functional and lint-compliant.packages/gazzodown/src/mentions/ChannelMentionElement.spec.tsx (1)
60-77: Drop the broad container snapshots here.These duplicate the behavioral checks above, and the resolved case snapshots
MessageHighlightstructure/title frompackages/gazzodown/src/mentions/ChannelMentionElement.tsx, so unrelated markup or i18n changes will create churn in this suite. I’d keep the explicit assertions and remove these snapshots, or narrow them to the minimal rendered text.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/mentions/ChannelMentionElement.spec.tsx` around lines 60 - 77, The tests currently use broad container snapshots for ChannelMentionElement which duplicate earlier behavioral assertions and couple to unrelated markup (e.g., MessageHighlight) and i18n; replace or remove these snapshots in the two tests so they only assert the minimal rendered output: for the resolved case use MarkupInteractionContext with the resolveChannelMention mock and assert the specific displayed text/label from ChannelMentionElement (not the full container snapshot), and for the unresolved case assert the exact fallback text rendered by ChannelMentionElement rather than matching container; keep references to ChannelMentionElement, MarkupInteractionContext and resolveChannelMention to locate the tests to change.packages/gazzodown/src/mentions/PreviewChannelMentionElement.spec.tsx (1)
16-18: This snapshot is redundant with the text assertions above.For a component that only exposes
#${mention}behavior, the snapshot adds maintenance cost without much extra coverage. Prefer the explicittoHaveTextContentassertions already used in this file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/mentions/PreviewChannelMentionElement.spec.tsx` around lines 16 - 18, Remove the redundant snapshot test in PreviewChannelMentionElement.spec.tsx: delete the "it('matches snapshot', ...)" block that renders PreviewChannelMentionElement and calls expect(container).toMatchSnapshot(), and either rely on the existing toHaveTextContent assertions already in the file or replace the snapshot assertion with expect(container).toHaveTextContent(`#${mention}`) to assert the rendered text explicitly.packages/gazzodown/src/elements/ItalicSpan.spec.tsx (2)
18-20: This snapshot is low-signal and likely to churn.It covers the same plain-text path as the first test, but snapshots the whole container, so incidental wrapper changes will break it. I'd replace it with a focused assertion here, and keep snapshots for richer nested-token trees.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/elements/ItalicSpan.spec.tsx` around lines 18 - 20, The snapshot test for ItalicSpan is low-signal and should be replaced with a focused assertion: update the test named 'matches snapshot' in ItalicSpan.spec.tsx to render <ItalicSpan>{[{ type: 'PLAIN_TEXT', value: 'Snapshot italic' }]}</ItalicSpan> and assert the specific output (e.g., that the rendered text equals "Snapshot italic" and/or that the token is wrapped in an <em> element) instead of snapshotting the entire container; locate the test referencing the ItalicSpan component and change the expect from toMatchSnapshot() to a precise assertion like checking container.textContent or querying for the <em> element.
6-15: Drop the inline lint suppressions in these assertions.
screen.getByText(...)already gives you the rendered element here, so these tests can assert the tag directly instead of usingclosest()pluseslint-disablecomments.Suggested cleanup
it('renders italic text with em tag', () => { render(<ItalicSpan>{[{ type: 'PLAIN_TEXT', value: 'Italic text' }]}</ItalicSpan>); - // eslint-disable-next-line testing-library/no-node-access - expect(screen.getByText('Italic text').closest('em')).toBeInTheDocument(); + expect(screen.getByText('Italic text').tagName).toBe('EM'); }); it('renders nested bold inside italic', () => { render(<ItalicSpan>{[{ type: 'BOLD', value: [{ type: 'PLAIN_TEXT', value: 'Bold inside italic' }] }]}</ItalicSpan>); - // eslint-disable-next-line testing-library/no-node-access - expect(screen.getByText('Bold inside italic').closest('strong')).toBeInTheDocument(); + expect(screen.getByText('Bold inside italic').tagName).toBe('STRONG'); });As per coding guidelines, "Avoid code comments in the implementation".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/elements/ItalicSpan.spec.tsx` around lines 6 - 15, The tests in ItalicSpan.spec.tsx should drop the inline eslint-disable comments and assert the element tag directly using the element returned by screen.getByText rather than calling closest(); update the first test (it('renders italic text with em tag')) to remove the eslint-disable line and assert screen.getByText('Italic text').tagName === 'EM' (or use expect(...).toBe('EM')/toEqual('EM') as your matcher style), and update the second test (it('renders nested bold inside italic')) to remove its eslint-disable and assert screen.getByText('Bold inside italic').tagName === 'STRONG' instead. Ensure you only modify the assertions in the tests around the ItalicSpan render calls and remove the commented disables.packages/gazzodown/src/code/PreviewCodeElement.spec.tsx (1)
16-18: Narrow the snapshot surface.Snapshotting
containerbakes the whole rendered tree into the expectation, so harmless wrapper/markup refactors will churn this test. Using a fragment snapshot here is usually less brittle.💡 Suggested change
- it('matches snapshot', () => { - const { container } = render(<PreviewCodeElement code='const x = 42;' />); - expect(container).toMatchSnapshot(); - }); + it('matches snapshot', () => { + const { asFragment } = render(<PreviewCodeElement code='const x = 42;' />); + expect(asFragment()).toMatchSnapshot(); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/code/PreviewCodeElement.spec.tsx` around lines 16 - 18, The test is snapshotting the entire render container which is brittle; update the 'matches snapshot' test for PreviewCodeElement to narrow the snapshot surface by using the render utility's asFragment() (or snapshot a specific element like the code node) instead of container, so only the component output is snapshotted and wrapper/markup changes won't break the test.packages/gazzodown/src/colors/ColorElement.spec.tsx (1)
24-26: Snapshot the focused component output or remove this redundant case.Lines 6–22 already assert all visible behavior using targeted
toBeInTheDocument()andtoHaveStyle()assertions. The snapshot at lines 24–26 captures the fullcontainer(including React Testing Library's wrapper<div>), which adds noise and churns when test infrastructure changes. Per React Testing Library and Jest documentation, snapshot onlycontainer.firstChildif a snapshot is needed, or drop this test since behavior is already verified.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/colors/ColorElement.spec.tsx` around lines 24 - 26, The snapshot test for ColorElement is capturing the entire render container (including RTL wrapper) which is noisy; either remove this redundant snapshot test since the visible behavior is already asserted by toBeInTheDocument/toHaveStyle, or update it to snapshot only the focused component by replacing the snapshot target with container.firstChild (i.e., keep the test for ColorElement but call expect(container.firstChild).toMatchSnapshot()). Locate the test named "matches snapshot" in ColorElement.spec.tsx and implement one of these two fixes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/gazzodown/src/blocks/TaskListBlock.spec.tsx`:
- Around line 23-32: The test currently only ensures onTaskChecked was called
during render; instead simulate a user toggling the checkbox rendered by
TaskListBlock and assert the handler returned from MarkupInteractionContext's
onTaskChecked is invoked with the correct task argument and that its returned
callback is executed; locate the test around TaskListBlock and
MarkupInteractionContext, render the component with a jest.fn for onTaskChecked
that returns a spy function, find the checkbox element (e.g. by role or label),
fire a change/click event on it, then assert onTaskChecked received the expected
task object and the returned callback spy was called.
In `@packages/gazzodown/src/code/PreviewCodeElement.spec.tsx`:
- Around line 11-13: The test "renders empty string without crashing" currently
only checks that render() returned a container; change it to assert the actual
rendered output for the empty-case instead. In PreviewCodeElement.spec.tsx
update the test for PreviewCodeElement (test name 'renders empty string without
crashing') to render(<PreviewCodeElement code='' />) and then assert the DOM
contains the expected empty-state (for example check container.textContent is ''
or that the rendered code/pre element from PreviewCodeElement has textContent
=== '' and that no fallback text is present) so the test fails if fallback
text/markup appears.
In `@packages/gazzodown/src/colors/ColorElement.spec.tsx`:
- Around line 13-16: The test currently reaches into DOM structure by calling
getByText(...) then querySelector('span') and asserts an exact serialized
backgroundColor string; update the component (ColorElement) to add a stable
data-testid (e.g., data-testid="color-swatch") on the swatch element and change
the spec to use screen.getByTestId('color-swatch') instead of querySelector;
also relax the assertion on backgroundColor (for example assert it contains the
expected rgba triplet or use a regex/partial match) rather than relying on the
exact percentage serialization so the test is resilient to markup or CSS
serialization changes.
In `@packages/gazzodown/src/elements/ItalicSpan.spec.tsx`:
- Around line 12-15: The test in ItalicSpan.spec.tsx currently only asserts the
nested <strong> exists, so change the assertion to ensure the bold node is
wrapped by an <em> from ItalicSpan: after rendering the <ItalicSpan> with nested
'BOLD', locate the text node via screen.getByText('Bold inside italic') and
assert its closest('em') is present (or that closest('em') contains the
closest('strong')), ensuring the outer italic wrapper rendered by ItalicSpan is
verified rather than only the inner strong.
In `@packages/gazzodown/src/elements/Timestamp/LongDate.tsx`:
- Line 9: The LongDate component is using the wrong date-fns format pattern
('Pp' includes time); update LongDate (function/component LongDate) to call
format(value, 'PP') and keep dateTime={value.toISOString()} so it renders a
medium localized date only (matching ShortDate's 'P' and FullDate's longer
patterns).
In `@packages/gazzodown/src/emoji/BigEmojiElement.spec.tsx`:
- Around line 7-33: Tests for BigEmojiElement never exercise the "big" branch
because no detectEmoji is provided, so the component always takes the fallback
<span> path; update the tests to drive the detected-emoji path by either (a)
providing a detectEmoji stub in the MarkupInteractionContext.Provider that
returns a value indicating an emoji was detected (so BigEmojiElement takes the
detected branch), or (b) mocking the Emoji component to simulate a detected
emoji being rendered; change the three tests to include the detectEmoji stub or
mock so the BigEmojiElement code path that depends on detectEmoji (and the
forwarding of the big prop to Emoji) is actually covered.
In `@packages/gazzodown/src/emoji/emoji.spec.tsx`:
- Around line 68-85: The tests currently assert snapshots but exercise the
unicode-fallback path where EmojiRenderer ignores big/preview, so they don't
verify Emoji forwards those props; update the tests to either (A) mock
./EmojiRenderer and assert the props received from the Emoji component (mock the
module and expect the mock to be called with big/preview), or (B) render Emoji
through the detected-emoji branch by providing a detected emoji (or stubbing the
detection behavior) so the DOM output changes when big/preview are set; locate
the test file using the Emoji component and the EmojiRenderer module references
and change the test implementations accordingly to assert forwarded props rather
than relying on the fallback snapshot.
In `@packages/gazzodown/src/PreviewMarkup.spec.tsx`:
- Around line 185-202: The test named "renders code block preview with first
line" only checks the first line exists but doesn't ensure truncation; update
the spec to also assert the second line is not rendered by using PreviewMarkup's
rendered output checks (e.g., ensure "const y = 2;" is absent via queryByText or
expect(...).not.toBeInTheDocument()) so the test fails if the component renders
the full code block; target the test block that renders <PreviewMarkup ...
tokens=[{ type: 'CODE', value: [...] }] /> and add the negative assertion for
the second CODE_LINE.
- Around line 71-107: Update the two tests in PreviewMarkup.spec.tsx to assert
the actual list prefixes instead of only the item text: in the "renders first
item of unordered list with dash prefix" test assert that container.textContent
contains the dash-prefixed string (e.g., "- First") and does not contain the
second item or its prefix; similarly in the "renders first item of ordered list
with number prefix" test assert that container.textContent contains the numbered
prefix (e.g., "1. First") and does not contain the second item or its prefix,
referencing the PreviewMarkup render call and container.textContent used in each
test.
---
Duplicate comments:
In `@packages/gazzodown/src/emoji/EmojiRenderer.spec.tsx`:
- Around line 37-38: Add a strict call-count assertion for the mock detectEmoji
in EmojiRenderer.spec.tsx: before the existing
expect(detectEmoji).toHaveBeenCalledWith(':smile:'), add
expect(detectEmoji).toHaveBeenCalledTimes(1) so the test fails if detectEmoji is
invoked more than once; locate the mock/spy named detectEmoji in the spec and
insert the call count assertion immediately prior to the toHaveBeenCalledWith
check.
---
Nitpick comments:
In `@packages/gazzodown/src/code/PreviewCodeElement.spec.tsx`:
- Around line 16-18: The test is snapshotting the entire render container which
is brittle; update the 'matches snapshot' test for PreviewCodeElement to narrow
the snapshot surface by using the render utility's asFragment() (or snapshot a
specific element like the code node) instead of container, so only the component
output is snapshotted and wrapper/markup changes won't break the test.
In `@packages/gazzodown/src/colors/ColorElement.spec.tsx`:
- Around line 24-26: The snapshot test for ColorElement is capturing the entire
render container (including RTL wrapper) which is noisy; either remove this
redundant snapshot test since the visible behavior is already asserted by
toBeInTheDocument/toHaveStyle, or update it to snapshot only the focused
component by replacing the snapshot target with container.firstChild (i.e., keep
the test for ColorElement but call
expect(container.firstChild).toMatchSnapshot()). Locate the test named "matches
snapshot" in ColorElement.spec.tsx and implement one of these two fixes.
In `@packages/gazzodown/src/elements/ItalicSpan.spec.tsx`:
- Around line 18-20: The snapshot test for ItalicSpan is low-signal and should
be replaced with a focused assertion: update the test named 'matches snapshot'
in ItalicSpan.spec.tsx to render <ItalicSpan>{[{ type: 'PLAIN_TEXT', value:
'Snapshot italic' }]}</ItalicSpan> and assert the specific output (e.g., that
the rendered text equals "Snapshot italic" and/or that the token is wrapped in
an <em> element) instead of snapshotting the entire container; locate the test
referencing the ItalicSpan component and change the expect from
toMatchSnapshot() to a precise assertion like checking container.textContent or
querying for the <em> element.
- Around line 6-15: The tests in ItalicSpan.spec.tsx should drop the inline
eslint-disable comments and assert the element tag directly using the element
returned by screen.getByText rather than calling closest(); update the first
test (it('renders italic text with em tag')) to remove the eslint-disable line
and assert screen.getByText('Italic text').tagName === 'EM' (or use
expect(...).toBe('EM')/toEqual('EM') as your matcher style), and update the
second test (it('renders nested bold inside italic')) to remove its
eslint-disable and assert screen.getByText('Bold inside italic').tagName ===
'STRONG' instead. Ensure you only modify the assertions in the tests around the
ItalicSpan render calls and remove the commented disables.
In `@packages/gazzodown/src/elements/Timestamp/RelativeTime.tsx`:
- Around line 10-25: Remove the inline explanatory comments in RelativeTime.tsx
that precede each if block (the comments describing "less than 1 minute",
"minutes range", "hours range", and "refresh the time in 1 hour") while keeping
the logic and return values intact; locate the conditional branch logic inside
the RelativeTime component/function (the timeToRefresh checks and their returns
1000, 60000/2, 60000*5, etc.) and delete only the comment lines so the code
remains functional and lint-compliant.
In `@packages/gazzodown/src/mentions/ChannelMentionElement.spec.tsx`:
- Around line 60-77: The tests currently use broad container snapshots for
ChannelMentionElement which duplicate earlier behavioral assertions and couple
to unrelated markup (e.g., MessageHighlight) and i18n; replace or remove these
snapshots in the two tests so they only assert the minimal rendered output: for
the resolved case use MarkupInteractionContext with the resolveChannelMention
mock and assert the specific displayed text/label from ChannelMentionElement
(not the full container snapshot), and for the unresolved case assert the exact
fallback text rendered by ChannelMentionElement rather than matching container;
keep references to ChannelMentionElement, MarkupInteractionContext and
resolveChannelMention to locate the tests to change.
In `@packages/gazzodown/src/mentions/PreviewChannelMentionElement.spec.tsx`:
- Around line 16-18: Remove the redundant snapshot test in
PreviewChannelMentionElement.spec.tsx: delete the "it('matches snapshot', ...)"
block that renders PreviewChannelMentionElement and calls
expect(container).toMatchSnapshot(), and either rely on the existing
toHaveTextContent assertions already in the file or replace the snapshot
assertion with expect(container).toHaveTextContent(`#${mention}`) to assert the
rendered text explicitly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1fc0957c-c393-4005-b6cd-fa113294fff5
⛔ Files ignored due to path filters (44)
packages/gazzodown/src/__snapshots__/Markup.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/__snapshots__/PreviewMarkup.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/blocks/__snapshots__/HeadingBlock.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/blocks/__snapshots__/OrderedListBlock.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/blocks/__snapshots__/ParagraphBlock.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/blocks/__snapshots__/QuoteBlock.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/blocks/__snapshots__/SpoilerBlock.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/blocks/__snapshots__/TaskListBlock.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/blocks/__snapshots__/UnorderedListBlock.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/code/__snapshots__/CodeBlock.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/code/__snapshots__/CodeElement.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/code/__snapshots__/PreviewCodeBlock.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/code/__snapshots__/PreviewCodeElement.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/colors/__snapshots__/ColorElement.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/colors/__snapshots__/PreviewColorElement.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/elements/Timestamp/__snapshots__/DateTimeFormats.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/elements/Timestamp/__snapshots__/RelativeTime.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/elements/Timestamp/__snapshots__/Time.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/elements/Timestamp/__snapshots__/Timestamp.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/elements/Timestamp/__snapshots__/TimestampWrapper.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/elements/__snapshots__/BoldSpan.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/elements/__snapshots__/ImageElement.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/elements/__snapshots__/InlineElements.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/elements/__snapshots__/ItalicSpan.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/elements/__snapshots__/LinkSpan.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/elements/__snapshots__/PlainSpan.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/elements/__snapshots__/PreviewInlineElements.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/elements/__snapshots__/SpoilerSpan.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/elements/__snapshots__/StrikeSpan.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/emoji/__snapshots__/BigEmojiBlock.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/emoji/__snapshots__/BigEmojiElement.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/emoji/__snapshots__/EmojiRenderer.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/emoji/__snapshots__/PreviewBigEmojiBlock.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/emoji/__snapshots__/PreviewEmojiElement.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/emoji/__snapshots__/emoji.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/katex/__snapshots__/KatexBlock.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/katex/__snapshots__/KatexElement.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/katex/__snapshots__/KatexErrorBoundary.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/katex/__snapshots__/PreviewKatexBlock.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/katex/__snapshots__/PreviewKatexElement.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/mentions/__snapshots__/ChannelMentionElement.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/mentions/__snapshots__/PreviewChannelMentionElement.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/mentions/__snapshots__/PreviewUserMentionElement.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/mentions/__snapshots__/UserMentionElement.spec.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (57)
packages/gazzodown/src/Markup.spec.tsxpackages/gazzodown/src/Markup.stories.tsxpackages/gazzodown/src/PreviewMarkup.spec.tsxpackages/gazzodown/src/blocks/HeadingBlock.spec.tsxpackages/gazzodown/src/blocks/OrderedListBlock.spec.tsxpackages/gazzodown/src/blocks/ParagraphBlock.spec.tsxpackages/gazzodown/src/blocks/QuoteBlock.spec.tsxpackages/gazzodown/src/blocks/SpoilerBlock.spec.tsxpackages/gazzodown/src/blocks/TaskListBlock.spec.tsxpackages/gazzodown/src/blocks/UnorderedListBlock.spec.tsxpackages/gazzodown/src/code/CodeBlock.spec.tsxpackages/gazzodown/src/code/CodeElement.spec.tsxpackages/gazzodown/src/code/PreviewCodeBlock.spec.tsxpackages/gazzodown/src/code/PreviewCodeElement.spec.tsxpackages/gazzodown/src/colors/ColorElement.spec.tsxpackages/gazzodown/src/colors/PreviewColorElement.spec.tsxpackages/gazzodown/src/elements/BoldSpan.spec.tsxpackages/gazzodown/src/elements/ImageElement.spec.tsxpackages/gazzodown/src/elements/InlineElements.spec.tsxpackages/gazzodown/src/elements/ItalicSpan.spec.tsxpackages/gazzodown/src/elements/LinkSpan.spec.tsxpackages/gazzodown/src/elements/PlainSpan.spec.tsxpackages/gazzodown/src/elements/PreviewInlineElements.spec.tsxpackages/gazzodown/src/elements/SpoilerSpan.spec.tsxpackages/gazzodown/src/elements/StrikeSpan.spec.tsxpackages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsxpackages/gazzodown/src/elements/Timestamp/FullDate.tsxpackages/gazzodown/src/elements/Timestamp/FullDateLong.tsxpackages/gazzodown/src/elements/Timestamp/LongDate.tsxpackages/gazzodown/src/elements/Timestamp/LongTime.tsxpackages/gazzodown/src/elements/Timestamp/RelativeTime.spec.tsxpackages/gazzodown/src/elements/Timestamp/RelativeTime.tsxpackages/gazzodown/src/elements/Timestamp/ShortDate.tsxpackages/gazzodown/src/elements/Timestamp/ShortTime.tsxpackages/gazzodown/src/elements/Timestamp/Time.spec.tsxpackages/gazzodown/src/elements/Timestamp/Time.tsxpackages/gazzodown/src/elements/Timestamp/Timestamp.spec.tsxpackages/gazzodown/src/elements/Timestamp/Timestamp.tsxpackages/gazzodown/src/elements/Timestamp/TimestampWrapper.spec.tsxpackages/gazzodown/src/elements/Timestamp/TimestampWrapper.tsxpackages/gazzodown/src/elements/Timestamp/index.tspackages/gazzodown/src/elements/Timestamp/index.tsxpackages/gazzodown/src/emoji/BigEmojiBlock.spec.tsxpackages/gazzodown/src/emoji/BigEmojiElement.spec.tsxpackages/gazzodown/src/emoji/EmojiRenderer.spec.tsxpackages/gazzodown/src/emoji/PreviewBigEmojiBlock.spec.tsxpackages/gazzodown/src/emoji/PreviewEmojiElement.spec.tsxpackages/gazzodown/src/emoji/emoji.spec.tsxpackages/gazzodown/src/katex/KatexBlock.spec.tsxpackages/gazzodown/src/katex/KatexElement.spec.tsxpackages/gazzodown/src/katex/KatexErrorBoundary.spec.tsxpackages/gazzodown/src/katex/PreviewKatexBlock.spec.tsxpackages/gazzodown/src/katex/PreviewKatexElement.spec.tsxpackages/gazzodown/src/mentions/ChannelMentionElement.spec.tsxpackages/gazzodown/src/mentions/PreviewChannelMentionElement.spec.tsxpackages/gazzodown/src/mentions/PreviewUserMentionElement.spec.tsxpackages/gazzodown/src/mentions/UserMentionElement.spec.tsx
💤 Files with no reviewable changes (1)
- packages/gazzodown/src/elements/Timestamp/index.tsx
🚧 Files skipped from review as they are similar to previous changes (38)
- packages/gazzodown/src/mentions/PreviewUserMentionElement.spec.tsx
- packages/gazzodown/src/elements/Timestamp/TimestampWrapper.spec.tsx
- packages/gazzodown/src/elements/SpoilerSpan.spec.tsx
- packages/gazzodown/src/blocks/ParagraphBlock.spec.tsx
- packages/gazzodown/src/elements/BoldSpan.spec.tsx
- packages/gazzodown/src/elements/Timestamp/ShortDate.tsx
- packages/gazzodown/src/katex/KatexBlock.spec.tsx
- packages/gazzodown/src/elements/Timestamp/FullDateLong.tsx
- packages/gazzodown/src/blocks/HeadingBlock.spec.tsx
- packages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsx
- packages/gazzodown/src/katex/PreviewKatexBlock.spec.tsx
- packages/gazzodown/src/blocks/QuoteBlock.spec.tsx
- packages/gazzodown/src/elements/InlineElements.spec.tsx
- packages/gazzodown/src/elements/Timestamp/Timestamp.tsx
- packages/gazzodown/src/Markup.spec.tsx
- packages/gazzodown/src/elements/Timestamp/Time.tsx
- packages/gazzodown/src/katex/KatexElement.spec.tsx
- packages/gazzodown/src/emoji/PreviewBigEmojiBlock.spec.tsx
- packages/gazzodown/src/mentions/UserMentionElement.spec.tsx
- packages/gazzodown/src/elements/Timestamp/Time.spec.tsx
- packages/gazzodown/src/blocks/OrderedListBlock.spec.tsx
- packages/gazzodown/src/katex/PreviewKatexElement.spec.tsx
- packages/gazzodown/src/blocks/SpoilerBlock.spec.tsx
- packages/gazzodown/src/elements/Timestamp/TimestampWrapper.tsx
- packages/gazzodown/src/code/CodeBlock.spec.tsx
- packages/gazzodown/src/elements/PlainSpan.spec.tsx
- packages/gazzodown/src/emoji/PreviewEmojiElement.spec.tsx
- packages/gazzodown/src/elements/LinkSpan.spec.tsx
- packages/gazzodown/src/elements/Timestamp/ShortTime.tsx
- packages/gazzodown/src/elements/Timestamp/RelativeTime.spec.tsx
- packages/gazzodown/src/elements/ImageElement.spec.tsx
- packages/gazzodown/src/elements/Timestamp/index.ts
- packages/gazzodown/src/elements/Timestamp/Timestamp.spec.tsx
- packages/gazzodown/src/elements/StrikeSpan.spec.tsx
- packages/gazzodown/src/code/PreviewCodeBlock.spec.tsx
- packages/gazzodown/src/elements/PreviewInlineElements.spec.tsx
- packages/gazzodown/src/emoji/BigEmojiBlock.spec.tsx
- packages/gazzodown/src/colors/PreviewColorElement.spec.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
packages/gazzodown/src/code/CodeElement.spec.tsxpackages/gazzodown/src/elements/ItalicSpan.spec.tsxpackages/gazzodown/src/blocks/UnorderedListBlock.spec.tsxpackages/gazzodown/src/colors/ColorElement.spec.tsxpackages/gazzodown/src/emoji/BigEmojiElement.spec.tsxpackages/gazzodown/src/Markup.stories.tsxpackages/gazzodown/src/code/PreviewCodeElement.spec.tsxpackages/gazzodown/src/mentions/PreviewChannelMentionElement.spec.tsxpackages/gazzodown/src/elements/Timestamp/LongDate.tsxpackages/gazzodown/src/elements/Timestamp/FullDate.tsxpackages/gazzodown/src/elements/Timestamp/LongTime.tsxpackages/gazzodown/src/PreviewMarkup.spec.tsxpackages/gazzodown/src/elements/Timestamp/RelativeTime.tsxpackages/gazzodown/src/blocks/TaskListBlock.spec.tsxpackages/gazzodown/src/katex/KatexErrorBoundary.spec.tsxpackages/gazzodown/src/emoji/EmojiRenderer.spec.tsxpackages/gazzodown/src/emoji/emoji.spec.tsxpackages/gazzodown/src/mentions/ChannelMentionElement.spec.tsx
🧠 Learnings (16)
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (`test`, `page`, `expect`) for consistency in test files
Applied to files:
packages/gazzodown/src/code/CodeElement.spec.tsxpackages/gazzodown/src/elements/ItalicSpan.spec.tsxpackages/gazzodown/src/emoji/BigEmojiElement.spec.tsxpackages/gazzodown/src/code/PreviewCodeElement.spec.tsxpackages/gazzodown/src/mentions/PreviewChannelMentionElement.spec.tsxpackages/gazzodown/src/PreviewMarkup.spec.tsxpackages/gazzodown/src/blocks/TaskListBlock.spec.tsxpackages/gazzodown/src/katex/KatexErrorBoundary.spec.tsxpackages/gazzodown/src/emoji/EmojiRenderer.spec.tsxpackages/gazzodown/src/emoji/emoji.spec.tsxpackages/gazzodown/src/mentions/ChannelMentionElement.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `expect` matchers for assertions (`toEqual`, `toContain`, `toBeTruthy`, `toHaveLength`, etc.) instead of `assert` statements in Playwright tests
Applied to files:
packages/gazzodown/src/code/CodeElement.spec.tsxpackages/gazzodown/src/blocks/UnorderedListBlock.spec.tsxpackages/gazzodown/src/emoji/BigEmojiElement.spec.tsxpackages/gazzodown/src/code/PreviewCodeElement.spec.tsxpackages/gazzodown/src/PreviewMarkup.spec.tsxpackages/gazzodown/src/blocks/TaskListBlock.spec.tsxpackages/gazzodown/src/emoji/EmojiRenderer.spec.tsxpackages/gazzodown/src/mentions/ChannelMentionElement.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to **/*.spec.ts : Use descriptive test names that clearly communicate expected behavior in Playwright tests
Applied to files:
packages/gazzodown/src/elements/ItalicSpan.spec.tsxpackages/gazzodown/src/blocks/UnorderedListBlock.spec.tsxpackages/gazzodown/src/emoji/BigEmojiElement.spec.tsxpackages/gazzodown/src/code/PreviewCodeElement.spec.tsxpackages/gazzodown/src/mentions/PreviewChannelMentionElement.spec.tsxpackages/gazzodown/src/PreviewMarkup.spec.tsxpackages/gazzodown/src/blocks/TaskListBlock.spec.tsxpackages/gazzodown/src/katex/KatexErrorBoundary.spec.tsxpackages/gazzodown/src/emoji/EmojiRenderer.spec.tsxpackages/gazzodown/src/emoji/emoji.spec.tsx
📚 Learning: 2025-12-10T21:00:54.909Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37091
File: ee/packages/abac/jest.config.ts:4-7
Timestamp: 2025-12-10T21:00:54.909Z
Learning: Rocket.Chat monorepo: Jest testMatch pattern '<rootDir>/src/**/*.spec.(ts|js|mjs)' is valid in this repo and used across multiple packages (e.g., packages/tools, ee/packages/omnichannel-services). Do not flag it as invalid in future reviews.
Applied to files:
packages/gazzodown/src/emoji/BigEmojiElement.spec.tsxpackages/gazzodown/src/mentions/PreviewChannelMentionElement.spec.tsxpackages/gazzodown/src/PreviewMarkup.spec.tsxpackages/gazzodown/src/blocks/TaskListBlock.spec.tsxpackages/gazzodown/src/emoji/EmojiRenderer.spec.tsxpackages/gazzodown/src/emoji/emoji.spec.tsxpackages/gazzodown/src/mentions/ChannelMentionElement.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `test.step()` for complex test scenarios to improve organization in Playwright tests
Applied to files:
packages/gazzodown/src/emoji/BigEmojiElement.spec.tsxpackages/gazzodown/src/code/PreviewCodeElement.spec.tsxpackages/gazzodown/src/PreviewMarkup.spec.tsxpackages/gazzodown/src/blocks/TaskListBlock.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Prefer web-first assertions (`toBeVisible`, `toHaveText`, etc.) in Playwright tests
Applied to files:
packages/gazzodown/src/emoji/BigEmojiElement.spec.tsxpackages/gazzodown/src/code/PreviewCodeElement.spec.tsxpackages/gazzodown/src/PreviewMarkup.spec.tsxpackages/gazzodown/src/blocks/TaskListBlock.spec.tsxpackages/gazzodown/src/emoji/EmojiRenderer.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Maintain test isolation between test cases in Playwright tests
Applied to files:
packages/gazzodown/src/code/PreviewCodeElement.spec.tsxpackages/gazzodown/src/PreviewMarkup.spec.tsxpackages/gazzodown/src/blocks/TaskListBlock.spec.tsxpackages/gazzodown/src/emoji/EmojiRenderer.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to **/*.{ts,tsx,js} : Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Applied to files:
packages/gazzodown/src/code/PreviewCodeElement.spec.tsxpackages/gazzodown/src/PreviewMarkup.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Implement proper wait strategies for dynamic content in Playwright tests
Applied to files:
packages/gazzodown/src/code/PreviewCodeElement.spec.tsx
📚 Learning: 2026-02-24T19:36:55.089Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/page-objects/fragments/home-content.ts:60-82
Timestamp: 2026-02-24T19:36:55.089Z
Learning: In RocketChat/Rocket.Chat e2e tests (apps/meteor/tests/e2e/page-objects/fragments/home-content.ts), thread message preview listitems do not have aria-roledescription="message", so lastThreadMessagePreview locator cannot be scoped to messageListItems (which filters for aria-roledescription="message"). It should remain scoped to page.getByRole('listitem') or mainMessageList.getByRole('listitem').
Applied to files:
packages/gazzodown/src/code/PreviewCodeElement.spec.tsxpackages/gazzodown/src/mentions/PreviewChannelMentionElement.spec.tsxpackages/gazzodown/src/PreviewMarkup.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure clean state for each test execution in Playwright tests
Applied to files:
packages/gazzodown/src/code/PreviewCodeElement.spec.tsxpackages/gazzodown/src/blocks/TaskListBlock.spec.tsx
📚 Learning: 2025-11-24T17:08:26.531Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/test-cases.mdc:0-0
Timestamp: 2025-11-24T17:08:26.531Z
Learning: Applies to **/*test-case*.md : Write test cases in Markdown format following the standardized template with ALL necessary components: Title, Description, Preconditions, Type, Steps, and Expected Result
Applied to files:
packages/gazzodown/src/PreviewMarkup.spec.tsx
📚 Learning: 2026-01-17T01:51:47.764Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38219
File: packages/core-typings/src/cloud/Announcement.ts:5-6
Timestamp: 2026-01-17T01:51:47.764Z
Learning: In packages/core-typings/src/cloud/Announcement.ts, the AnnouncementSchema.createdBy field intentionally overrides IBannerSchema.createdBy (object with _id and optional username) with a string enum ['cloud', 'system'] to match existing runtime behavior. This is documented as technical debt with a FIXME comment at apps/meteor/app/cloud/server/functions/syncWorkspace/handleCommsSync.ts:53 and should not be flagged as an error until the runtime behavior is corrected.
Applied to files:
packages/gazzodown/src/elements/Timestamp/RelativeTime.tsx
📚 Learning: 2025-11-19T12:32:29.696Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37547
File: packages/i18n/src/locales/en.i18n.json:634-634
Timestamp: 2025-11-19T12:32:29.696Z
Learning: Repo: RocketChat/Rocket.Chat
Context: i18n workflow
Learning: In this repository, new translation keys should be added to packages/i18n/src/locales/en.i18n.json only; other locale files are populated via the external translation pipeline and/or fall back to English. Do not request adding the same key to all locale files in future reviews.
Applied to files:
packages/gazzodown/src/elements/Timestamp/RelativeTime.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Group related tests in the same file
Applied to files:
packages/gazzodown/src/blocks/TaskListBlock.spec.tsxpackages/gazzodown/src/katex/KatexErrorBoundary.spec.tsxpackages/gazzodown/src/emoji/EmojiRenderer.spec.tsxpackages/gazzodown/src/emoji/emoji.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
packages/gazzodown/src/blocks/TaskListBlock.spec.tsxpackages/gazzodown/src/emoji/EmojiRenderer.spec.tsx
🔇 Additional comments (13)
packages/gazzodown/src/code/CodeElement.spec.tsx (1)
1-22: LGTM!The tests are concise, well-structured, and follow Testing Library best practices. Good coverage of:
- Basic rendering verification
- DOM structure and class assertions
- Snapshot for regression detection
packages/gazzodown/src/elements/Timestamp/LongTime.tsx (1)
1-11: LGTM!Clean implementation following the established timestamp component pattern. The
'pp'format token correctly outputs localized time with seconds, and thedateTimeattribute provides proper semantic markup for accessibility.packages/gazzodown/src/katex/KatexErrorBoundary.spec.tsx (4)
5-10: Well-structured test helper component.The
ThrowingComponenthelper is cleanly implemented with proper typing and serves its purpose for testing error boundary behavior.
13-19: Appropriate console.error suppression for error boundary tests.Mocking
console.erroris standard practice when testing React error boundaries, as React intentionally logs caught errors to the console. The cleanup inafterEachensures proper test isolation.
21-50: Comprehensive behavioral coverage with descriptive test names.The tests accurately verify the component's contract:
- Children render when no error occurs
- Fallback displays the
codeprop text when an error is caught- Error message is correctly set as the
titleattributeThese assertions align with the actual implementation where
<Fallback title={error?.message}>{code}</Fallback>is rendered on error. Based on learnings: "Use descriptive test names that clearly communicate expected behavior in Playwright tests" — this applies equally well here.
52-68: Snapshot tests cover both success and error states.The snapshot tests provide regression protection for the component's rendered output in both scenarios, aligning with the PR objective of adding snapshot test coverage. Note that as mentioned in the PR description, snapshot tests can become fragile to side changes — consider this tradeoff if the component's styling changes frequently.
packages/gazzodown/src/Markup.stories.tsx (1)
252-256: LGTM!The explicit parameter typing is accurate and makes the expected prop shape immediately clear at the function definition, which aids readability even though the type is already provided by
StoryFn<{ msg: string }>.packages/gazzodown/src/elements/Timestamp/RelativeTime.tsx (5)
7-27: Negative time difference causes inefficient refresh for past timestamps.The calculation
time - Date.now()yields a negative value for past timestamps, always triggering the 1-second refresh interval. UsingMath.abs()would apply the same cadence logic to both past and future timestamps.
37-47: Locale handling regresses and state naming is inconsistent.Multiple issues flagged previously:
- State setter
setTimeshould besetTextto match thetextstate variable.- Line 42 hardcodes
'en'instead of usinglanguage ?? 'en'.- Dependency array includes
valuebut omitslanguage, so language changes won't re-trigger the effect.
1-6: LGTM!Imports are appropriate and well-organized.
29-31: LGTM!Type definition is clean and correctly typed.
49-50: LGTM!Render logic correctly passes the computed text and ISO datetime to the Time component.
packages/gazzodown/src/elements/Timestamp/FullDate.tsx (1)
5-11: Looks good.The
Date-typed prop and ISOdateTimeattribute keep the component API and rendered markup aligned.
Comment on lines
+23
to
+32
| it('calls onTaskChecked from context when provided', () => { | ||
| const onTaskChecked = jest.fn(() => () => undefined); | ||
| render( | ||
| <MarkupInteractionContext.Provider value={{ onTaskChecked }}> | ||
| <TaskListBlock tasks={[{ type: 'TASK', status: false, value: [{ type: 'PLAIN_TEXT', value: 'Task' }] }]} /> | ||
| </MarkupInteractionContext.Provider>, | ||
| ); | ||
|
|
||
| expect(onTaskChecked).toHaveBeenCalled(); | ||
| }); |
Contributor
There was a problem hiding this comment.
Exercise the checkbox change, not just handler creation.
onTaskChecked is invoked during render to build the checkbox onChange callback, so this assertion passes even if the checkbox stops firing the returned handler. Trigger the checkbox and assert both the task argument and the returned callback are used.
🔧 Suggested change
it('calls onTaskChecked from context when provided', () => {
- const onTaskChecked = jest.fn(() => () => undefined);
+ const handleChange = jest.fn();
+ const onTaskChecked = jest.fn(() => handleChange);
render(
<MarkupInteractionContext.Provider value={{ onTaskChecked }}>
<TaskListBlock tasks={[{ type: 'TASK', status: false, value: [{ type: 'PLAIN_TEXT', value: 'Task' }] }]} />
</MarkupInteractionContext.Provider>,
);
- expect(onTaskChecked).toHaveBeenCalled();
+ screen.getByRole('checkbox').click();
+
+ expect(onTaskChecked).toHaveBeenCalledWith(
+ expect.objectContaining({ type: 'TASK', status: false }),
+ );
+ expect(handleChange).toHaveBeenCalled();
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/gazzodown/src/blocks/TaskListBlock.spec.tsx` around lines 23 - 32,
The test currently only ensures onTaskChecked was called during render; instead
simulate a user toggling the checkbox rendered by TaskListBlock and assert the
handler returned from MarkupInteractionContext's onTaskChecked is invoked with
the correct task argument and that its returned callback is executed; locate the
test around TaskListBlock and MarkupInteractionContext, render the component
with a jest.fn for onTaskChecked that returns a spy function, find the checkbox
element (e.g. by role or label), fire a change/click event on it, then assert
onTaskChecked received the expected task object and the returned callback spy
was called.
Comment on lines
+11
to
+13
| it('renders empty string without crashing', () => { | ||
| const { container } = render(<PreviewCodeElement code='' />); | ||
| expect(container).toBeInTheDocument(); |
Contributor
There was a problem hiding this comment.
Make the empty-string test assert output, not just successful render.
container exists whenever render() succeeds, so this does not verify the empty-state behavior. A regression that renders fallback text or unexpected markup would still pass here.
💡 Suggested change
- it('renders empty string without crashing', () => {
- const { container } = render(<PreviewCodeElement code='' />);
- expect(container).toBeInTheDocument();
- });
+ it('renders empty code without fallback content', () => {
+ const { container } = render(<PreviewCodeElement code='' />);
+ expect(container.firstChild).toBeInTheDocument();
+ expect(container.textContent).toBe('');
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| it('renders empty string without crashing', () => { | |
| const { container } = render(<PreviewCodeElement code='' />); | |
| expect(container).toBeInTheDocument(); | |
| it('renders empty code without fallback content', () => { | |
| const { container } = render(<PreviewCodeElement code='' />); | |
| expect(container.firstChild).toBeInTheDocument(); | |
| expect(container.textContent).toBe(''); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/gazzodown/src/code/PreviewCodeElement.spec.tsx` around lines 11 -
13, The test "renders empty string without crashing" currently only checks that
render() returned a container; change it to assert the actual rendered output
for the empty-case instead. In PreviewCodeElement.spec.tsx update the test for
PreviewCodeElement (test name 'renders empty string without crashing') to
render(<PreviewCodeElement code='' />) and then assert the DOM contains the
expected empty-state (for example check container.textContent is '' or that the
rendered code/pre element from PreviewCodeElement has textContent === '' and
that no fallback text is present) so the test fails if fallback text/markup
appears.
Comment on lines
+13
to
+16
| const text = screen.getByText(/rgba\(128, 64, 32/); | ||
| // eslint-disable-next-line testing-library/no-node-access | ||
| const swatch = text.querySelector('span'); | ||
| expect(swatch).toHaveStyle({ backgroundColor: 'rgba(128, 64, 32, 50.19607843137255%)' }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find and examine the test file
find . -name "ColorElement.spec.tsx" -type fRepository: RocketChat/Rocket.Chat
Length of output: 119
🏁 Script executed:
# Get the full context of the test file to understand the code
cat -n packages/gazzodown/src/colors/ColorElement.spec.tsxRepository: RocketChat/Rocket.Chat
Length of output: 1275
🌐 Web query:
Testing Library React 16.x best practices testing non-text decorative elements data-testid
💡 Result:
Best practices (React Testing Library / Testing Library)
- Prefer user-facing, semantic queries (what a user/assistive tech can perceive) and treat
data-testidas a last resort fallback. Testing Library explicitly recommends falling back to test ids only when you can’t use more semantic queries. [1] - Don’t test purely decorative UI “just because it’s there.” If an icon/spinner/divider is decorative, tests are usually better focused on behavior/state (e.g., “Save button disabled while loading”) rather than “the SVG exists”.
Non-text elements: what to do instead of data-testid
A) If the element is meaningful (should be accessible)
Make it accessible, then query it semantically.
Icon-only button
<button aria-label="Open settings">
<SettingsIcon />
</button>screen.getByRole('button', { name: /open settings/i })getByRole(..., { name }) is the recommended style over text-only queries when possible, and it uses the accessible name (e.g., aria-label). [2]
Standalone SVG/icon meant to convey meaning
Give it an accessible name and role, then query as an image:
<svg role="img" aria-label="Warning" />screen.getByRole('img', { name: /warning/i })(Again, this is relying on getByRole + accessible name.) [2]
B) If the element is purely decorative
Hide it from assistive tech:
<svg aria-hidden="true" />That’s a standard accessibility pattern for decorative icons. [3]
In that case, querying it with getByRole will often not work because it’s not in the accessibility tree; Testing Library provides a hidden: true option when you explicitly want to include normally-excluded elements. [2]
Example (only if you truly need to assert it exists):
screen.getByRole('img', { hidden: true }) // if it has role=img, etc.When data-testid is appropriate
Use data-testid only when there’s no stable user-facing handle (no role/name/label/text/alt) and you have a real reason to assert on that exact element (often rare for decorative elements). Testing Library documents ByTestId for exactly this fallback scenario. [4]
Example:
<svg data-testid="chevron-icon" aria-hidden="true" />expect(screen.getByTestId('chevron-icon')).toBeInTheDocument()Quick rule of thumb
- Meaningful element → make it accessible →
getByRole(..., { name })[2] - Decorative element →
aria-hidden="true"/ presentational → don’t test it directly; if you must, considerhidden: trueor (last resort)data-testid. [2][4]
Sources: Testing Library query guidance & fallback philosophy [1], getByRole and hidden: true option [2], ARIA guidance for decorative icons (aria-hidden) [3], ByTestId docs [4].
Use data-testid on the swatch element instead of querySelector.
Lines 13–16 couple the test to DOM structure (the span nesting) and exact CSS serialization (50.19607843137255%). When the component wraps the swatch differently or CSS serialization changes, the test breaks without any actual behavior change. Per Testing Library guidance, use a stable data-testid on the swatch element and query it with getByTestId() instead of traversing the DOM tree. This keeps the test focused on the rendered swatch and its styling, independent of markup details.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/gazzodown/src/colors/ColorElement.spec.tsx` around lines 13 - 16,
The test currently reaches into DOM structure by calling getByText(...) then
querySelector('span') and asserts an exact serialized backgroundColor string;
update the component (ColorElement) to add a stable data-testid (e.g.,
data-testid="color-swatch") on the swatch element and change the spec to use
screen.getByTestId('color-swatch') instead of querySelector; also relax the
assertion on backgroundColor (for example assert it contains the expected rgba
triplet or use a regex/partial match) rather than relying on the exact
percentage serialization so the test is resilient to markup or CSS serialization
changes.
Comment on lines
+68
to
+85
| it('passes big prop to EmojiRenderer', () => { | ||
| const { container } = render( | ||
| <MarkupInteractionContext.Provider value={{ convertAsciiToEmoji: true, useEmoji: true }}> | ||
| <Emoji big type='EMOJI' value={undefined} unicode='🔥' /> | ||
| </MarkupInteractionContext.Provider>, | ||
| ); | ||
|
|
||
| expect(container).toMatchSnapshot(); | ||
| }); | ||
|
|
||
| it('passes preview prop to EmojiRenderer', () => { | ||
| const { container } = render( | ||
| <MarkupInteractionContext.Provider value={{ convertAsciiToEmoji: true, useEmoji: true }}> | ||
| <Emoji preview type='EMOJI' value={undefined} unicode='🎉' /> | ||
| </MarkupInteractionContext.Provider>, | ||
| ); | ||
|
|
||
| expect(container).toMatchSnapshot(); |
Contributor
There was a problem hiding this comment.
These snapshots don't prove big or preview were forwarded.
Both tests render the unicode fallback path, and packages/gazzodown/src/emoji/EmojiRenderer.tsx:23-31,41-43 ignores big and preview there. If Emoji stopped passing either prop, these snapshots would still pass. Consider mocking ./EmojiRenderer and asserting the received props, or drive the detected-emoji branch so the DOM changes when those props are applied.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/gazzodown/src/emoji/emoji.spec.tsx` around lines 68 - 85, The tests
currently assert snapshots but exercise the unicode-fallback path where
EmojiRenderer ignores big/preview, so they don't verify Emoji forwards those
props; update the tests to either (A) mock ./EmojiRenderer and assert the props
received from the Emoji component (mock the module and expect the mock to be
called with big/preview), or (B) render Emoji through the detected-emoji branch
by providing a detected emoji (or stubbing the detection behavior) so the DOM
output changes when big/preview are set; locate the test file using the Emoji
component and the EmojiRenderer module references and change the test
implementations accordingly to assert forwarded props rather than relying on the
fallback snapshot.
fc2e45c to
c27da70
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (8)
packages/gazzodown/src/blocks/SpoilerBlock.spec.tsx (1)
30-36:⚠️ Potential issue | 🟡 MinorAssert the revealed content in the Space-key path.
This test still only verifies that the trigger disappears. It can pass even if the Space handler removes the button without rendering the spoiler body, so it should assert the revealed text too.
Suggested fix
it('reveals content on Space key', async () => { render(<SpoilerBlock>{[{ type: 'PARAGRAPH', value: [{ type: 'PLAIN_TEXT', value: 'Secret' }] }]}</SpoilerBlock>); screen.getByRole('button').focus(); await userEvent.keyboard(' '); expect(screen.queryByRole('button')).not.toBeInTheDocument(); + expect(screen.getByText('Secret')).toBeInTheDocument(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/blocks/SpoilerBlock.spec.tsx` around lines 30 - 36, The test "reveals content on Space key" in SpoilerBlock.spec.tsx currently only asserts the trigger button disappears; extend it to also assert the revealed content is rendered by checking for the spoiler text (e.g., "Secret") after simulating the Space key. After calling userEvent.keyboard(' '), add an assertion (using screen.getByText or screen.findByText) that the paragraph text from the rendered SpoilerBlock is present, while keeping the existing check that the button is not in the document.packages/gazzodown/src/blocks/TaskListBlock.spec.tsx (1)
23-32:⚠️ Potential issue | 🟠 MajorExercise the checkbox change instead of only the render-time callback factory.
onTaskCheckedis evaluated while renderingTaskListBlock, so this passes even if the checkbox never fires the returned handler. Toggle the checkbox and assert both the task argument and the returned callback are used.Suggested fix
- it('calls onTaskChecked from context when provided', () => { - const onTaskChecked = jest.fn(() => () => undefined); + it('calls the returned change handler when the checkbox is toggled', () => { + const task = { type: 'TASK', status: false, value: [{ type: 'PLAIN_TEXT', value: 'Task' }] } as const; + const handleChange = jest.fn(); + const onTaskChecked = jest.fn(() => handleChange); render( <MarkupInteractionContext.Provider value={{ onTaskChecked }}> - <TaskListBlock tasks={[{ type: 'TASK', status: false, value: [{ type: 'PLAIN_TEXT', value: 'Task' }] }]} /> + <TaskListBlock tasks={[task]} /> </MarkupInteractionContext.Provider>, ); - expect(onTaskChecked).toHaveBeenCalled(); + screen.getByRole('checkbox').click(); + + expect(onTaskChecked).toHaveBeenCalledWith(task); + expect(handleChange).toHaveBeenCalled(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/blocks/TaskListBlock.spec.tsx` around lines 23 - 32, The test currently only verifies that the onTaskChecked factory is accessed during render; update TaskListBlock.spec.tsx to simulate a user toggling the checkbox so the returned handler is executed: mock onTaskChecked as jest.fn().mockReturnValue(jest.fn()) (or capture the returned handler with a second mock), render TaskListBlock inside MarkupInteractionContext.Provider, locate the checkbox (e.g., getByRole('checkbox') or getByLabelText) and fire a click (userEvent.click or fireEvent.click), then assert onTaskChecked was called with the task object and that the returned handler mock was invoked.packages/gazzodown/src/Markup.spec.tsx (1)
285-291:⚠️ Potential issue | 🟡 MinorSnapshot a sanitized clone instead of mutating the live render tree.
Removing
<math>fromcontainerchanges the DOM under test before snapshotting and can mask regressions in the actual render output. Clone first, sanitize the clone, and snapshot that copy instead.Suggested fix
// eslint-disable-next-line testing-library/no-container, testing-library/no-node-access await waitFor(() => expect(container.querySelector('math')).toBeTruthy()); - // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access - container.querySelector('math')?.remove(); + const snapshotContainer = container.cloneNode(true) as HTMLElement; + // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access + snapshotContainer.querySelector('math')?.remove(); expect(screen.getByRole('math', { name: 'x^2 + y^2 = z^2' })).toBeInTheDocument(); - expect(container).toMatchSnapshot(); + expect(snapshotContainer).toMatchSnapshot();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/Markup.spec.tsx` around lines 285 - 291, The test currently mutates the live render tree by calling container.querySelector('math')?.remove() before snapshotting; instead, create a deep clone of the rendered container (use container.cloneNode(true)), remove or sanitize the <math> node(s) from that clone (e.g., clonedContainer.querySelector('math')?.remove()), and call expect(clonedContainer).toMatchSnapshot() so the live DOM (and subsequent assertions like screen.getByRole) remain unchanged; update the test around container.querySelector('math')?.remove() to operate on the cloned node.packages/gazzodown/src/elements/Timestamp/RelativeTime.spec.tsx (1)
49-61:⚠️ Potential issue | 🟡 MinorMake the interval test prove that React state changed.
This still passes if the interval callback never updates the rendered output. Advance timers inside
act()and assert the post-tick text differs frominitialText.Suggested fix
-import { render } from '@testing-library/react'; +import { act, render } from '@testing-library/react'; @@ it('updates text on interval', () => { const value = new Date('2025-06-15T11:59:30.000Z'); const { container } = render(<RelativeTime value={value} />); const initialText = container.textContent; - jest.advanceTimersByTime(2000); + act(() => { + jest.advanceTimersByTime(61_000); + }); - // Text may update after interval fires - expect(container.textContent).toBeTruthy(); - // The initial text should have been set expect(initialText).toBeTruthy(); + expect(container.textContent).toBeTruthy(); + expect(container.textContent).not.toBe(initialText); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/elements/Timestamp/RelativeTime.spec.tsx` around lines 49 - 61, The test for RelativeTime doesn't verify that the interval actually changed React state; wrap jest.advanceTimersByTime(2000) in React's act() (or the testing-library act helper) to flush effects, then assert that container.textContent !== initialText (i.e., the post-tick text differs from the saved initialText) to prove the component updated; locate the test case using the RelativeTime component and the initialText variable to apply this change.packages/gazzodown/src/elements/Timestamp/Timestamp.spec.tsx (2)
56-67:⚠️ Potential issue | 🟠 MajorAlways restore fake timers, even on assertion failure.
If the assertion on Line 64 throws,
jest.useRealTimers()never runs and the suite can leak fake timers into later tests. Wrap the fake-timer block intry/finallyor move cleanup toafterEach.Suggested fix
it('renders RelativeTime for format "R"', () => { jest.useFakeTimers(); - jest.setSystemTime(new Date('2025-06-15T15:00:00.000Z')); - - const { container } = render(<Timestamp format='R' value={date} />); - - // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access - const timeEl = container.querySelector('time'); - expect(timeEl).toHaveAttribute('dateTime', date.toISOString()); - - jest.useRealTimers(); + try { + jest.setSystemTime(new Date('2025-06-15T15:00:00.000Z')); + + const { container } = render(<Timestamp format='R' value={date} />); + + // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access + const timeEl = container.querySelector('time'); + expect(timeEl).toHaveAttribute('dateTime', date.toISOString()); + } finally { + jest.useRealTimers(); + } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/elements/Timestamp/Timestamp.spec.tsx` around lines 56 - 67, The test uses jest.useFakeTimers()/jest.setSystemTime around rendering Timestamp (render(<Timestamp format='R' value={date} />)) but restores timers with jest.useRealTimers() only at the end, so a failing assertion can leak fake timers; wrap the fake-timer block in a try/finally (call jest.useRealTimers() in finally) or move timer cleanup to an afterEach that always calls jest.useRealTimers() to guarantee restoration even on assertion failure.
76-79:⚠️ Potential issue | 🟡 MinorAvoid snapshotting host-localized timestamp markup here.
This snapshot is still environment-dependent because
Timerenders atitlevianew Date(dateTime).toLocaleString(). That makes the snapshot vary with locale/timezone while duplicating the deterministicdateTimeassertion already covered above.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/elements/Timestamp/Timestamp.spec.tsx` around lines 76 - 79, The test for Timestamp('f') should avoid environment-dependent snapshotting of host-localized markup produced by Time (which uses new Date(...).toLocaleString()); update the test in Timestamp.spec.tsx to stop using toMatchSnapshot and instead assert deterministic pieces: render <Timestamp format='f' value={date} />, find the Time/Timestamp element (by role/testid/text), assert its dateTime attribute equals the original ISO value and assert the visible text/format is as expected (or mock Intl/Date.prototype.toLocaleString if you need a specific string); reference the Timestamp component and the Time-rendered title to locate where to change the test.packages/gazzodown/src/elements/Timestamp/RelativeTime.tsx (2)
7-27:⚠️ Potential issue | 🟠 MajorPast timestamps fall into the 1-second refresh bucket forever.
For any past value,
time - Date.now()is negative, so Line 11 always matches and old timestamps keep re-rendering every second. Use the absolute delta before applying the thresholds.Suggested fix
const getTimeToRefresh = (time: number): number => { - const timeToRefresh = time - Date.now(); + const timeToRefresh = Math.abs(time - Date.now());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/elements/Timestamp/RelativeTime.tsx` around lines 7 - 27, getTimeToRefresh currently uses a signed delta (time - Date.now()), so past timestamps yield negative values and always hit the "< 60000" branch; change it to use the absolute difference (e.g., Math.abs(time - Date.now())) before evaluating thresholds so past times fall into the proper refresh buckets; update the logic inside getTimeToRefresh in RelativeTime.tsx accordingly and keep the same return values for each threshold.
37-47:⚠️ Potential issue | 🟠 MajorKeep interval refreshes on the active locale.
Initial render uses
language ?? 'en', but Line 42 hardcodes'en'and the effect does not depend onlanguage. A locale change after mount will either never refresh the text or be overwritten in English on the next tick.Suggested fix
const RelativeTime = ({ value }: RelativeTimeProps) => { const time = value.getTime(); const { language } = useContext(MarkupInteractionContext); - const [text, setTime] = useState(() => intlFormatDistance(time, Date.now(), { locale: language ?? 'en' })); + const locale = language ?? 'en'; + const [text, setText] = useState(() => intlFormatDistance(time, Date.now(), { locale })); const [timeToRefresh, setTimeToRefresh] = useState(() => getTimeToRefresh(time)); useEffect(() => { const interval = setInterval(() => { - setTime(intlFormatDistance(value.getTime(), Date.now(), { locale: 'en' })); + setText(intlFormatDistance(time, Date.now(), { locale })); setTimeToRefresh(getTimeToRefresh(time)); }, timeToRefresh); return () => clearInterval(interval); - }, [time, timeToRefresh, value]); + }, [locale, time, timeToRefresh]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/elements/Timestamp/RelativeTime.tsx` around lines 37 - 47, The effect hardcodes locale to 'en' and misses language in its deps, and it uses the stale local variable time when computing the next refresh; update the effect to call intlFormatDistance(value.getTime(), Date.now(), { locale: language ?? 'en' }) and setTimeToRefresh(getTimeToRefresh(value)) inside the interval, and add language to the useEffect dependency array (keep value, language, and timeToRefresh as needed) so locale changes trigger correct refreshes; verify initial useState calls (text/setTime and timeToRefresh/setTimeToRefresh) continue to use language ?? 'en' and getTimeToRefresh(value) consistently.
🧹 Nitpick comments (7)
packages/gazzodown/src/mentions/PreviewUserMentionElement.spec.tsx (1)
6-14: Same consolidation opportunity as channel mention tests.Both test cases verify the
@prefix rendering. The sameit.eachpattern could reduce duplication.♻️ Optional: Parameterized test approach
- it('renders user mention with @ prefix', () => { - const { container } = render(<PreviewUserMentionElement mention='john.doe' />); - expect(container).toHaveTextContent('@john.doe'); - }); - - it('renders different usernames', () => { - const { container } = render(<PreviewUserMentionElement mention='admin' />); - expect(container).toHaveTextContent('@admin'); - }); + it.each(['john.doe', 'admin', 'rocket.cat'])('renders user mention "%s" with @ prefix', (mention) => { + const { container } = render(<PreviewUserMentionElement mention={mention} />); + expect(container).toHaveTextContent(`@${mention}`); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/mentions/PreviewUserMentionElement.spec.tsx` around lines 6 - 14, Consolidate the two duplicate tests in PreviewUserMentionElement.spec.tsx by replacing the separate it blocks with a parameterized test (e.g., using it.each) that iterates over usernames and asserts the component renders the '@' prefixed value; update tests referencing PreviewUserMentionElement and the render + expect(container).toHaveTextContent('@{username}') pattern so a single table drives cases like 'john.doe' and 'admin'.packages/gazzodown/src/mentions/PreviewChannelMentionElement.spec.tsx (1)
6-14: Consider consolidating similar test cases or using parameterized tests.Tests "renders channel mention with # prefix" and "renders different channel names" verify identical behavior—both confirm that the
#prefix is rendered. If the intent is to demonstrate coverage across different inputs, consider usingit.eachfor clearer parameterization.♻️ Optional: Parameterized test approach
- it('renders channel mention with # prefix', () => { - const { container } = render(<PreviewChannelMentionElement mention='general' />); - expect(container).toHaveTextContent('#general'); - }); - - it('renders different channel names', () => { - const { container } = render(<PreviewChannelMentionElement mention='random-channel' />); - expect(container).toHaveTextContent('#random-channel'); - }); + it.each(['general', 'random-channel', 'announcements'])('renders channel mention "%s" with # prefix', (mention) => { + const { container } = render(<PreviewChannelMentionElement mention={mention} />); + expect(container).toHaveTextContent(`#${mention}`); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/mentions/PreviewChannelMentionElement.spec.tsx` around lines 6 - 14, The two tests for PreviewChannelMentionElement both assert the same behavior; replace them with a parameterized test using Jest's it.each to iterate over channel names (e.g., 'general', 'random-channel') and assert container has text '#<name>'; update the spec to remove the duplicate it blocks and add a single it.each that renders PreviewChannelMentionElement with the parameterized mention prop and checks toHaveTextContent(`#${mention}`).packages/gazzodown/src/elements/InlineElements.spec.tsx (1)
31-134: Consider adding tests for SPOILER, INLINE_KATEX, and TIMESTAMP cases.The test suite covers most inline element types, but comparing against the implementation in
InlineElements.tsx, these cases are missing test coverage:
SPOILER- renders<SpoilerSpan>INLINE_KATEX- renders<KatexElement>wrapped in<KatexErrorBoundary>TIMESTAMP- renders<Timestamp>Given the PR objective mentions tests becoming "fragile to side changes," you may have intentionally omitted these. If so, consider leaving a TODO comment for future coverage.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/elements/InlineElements.spec.tsx` around lines 31 - 134, Tests are missing coverage for SPOILER, INLINE_KATEX, and TIMESTAMP inline element cases; add unit tests in InlineElements.spec.tsx that exercise InlineElements (or the existing helper renderInlineElements) with: a SPOILER element verifying it renders a SpoilerSpan; an INLINE_KATEX element verifying it renders a KatexElement wrapped by a KatexErrorBoundary (use Suspense/fallback if needed); and a TIMESTAMP element verifying it renders a Timestamp component; keep tests consistent with existing patterns (use screen queries like getByText/getByRole or closest checks) and mirror how other cases (e.g., BOLD, EMOJI) are asserted.packages/gazzodown/src/elements/PreviewInlineElements.spec.tsx (1)
6-124: Consider adding test for INLINE_KATEX token type.The implementation handles
INLINE_KATEX(lines 65-69 in PreviewInlineElements.tsx) withKatexErrorBoundaryandPreviewKatexElement, but there's no test coverage for this token type. Given that KaTeX rendering involves an error boundary, testing both success and error cases would be valuable.💡 Suggested test case
it('renders INLINE_KATEX', () => { render( <PreviewInlineElements> {[{ type: 'INLINE_KATEX', value: 'x^2' }]} </PreviewInlineElements>, ); // Verify KaTeX output is rendered expect(screen.getByText(/x/)).toBeInTheDocument(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/elements/PreviewInlineElements.spec.tsx` around lines 6 - 124, Add a unit test for the INLINE_KATEX branch in PreviewInlineElements: render <PreviewInlineElements> with an element of type 'INLINE_KATEX' (value e.g. 'x^2') and assert the expected KaTeX output appears; also add a separate test that simulates a KaTeX render error (e.g. by mocking PreviewKatexElement or forcing KatexErrorBoundary to throw) and assert the error boundary fallback is shown. Target the PreviewInlineElements spec and reference the INLINE_KATEX token, KatexErrorBoundary and PreviewKatexElement when locating the implementation to cover both success and error cases.packages/gazzodown/src/elements/LinkSpan.spec.tsx (1)
65-75: Narrow these snapshots or drop them.These container snapshots mostly duplicate the explicit assertions above and will churn on unrelated DOM changes around the link. Snapshotting the anchor itself, or keeping only the behavioral assertions, will make this suite less fragile.
Possible simplification
- it('matches snapshot for external link', () => { - const { container } = render(<LinkSpan href='https://example.com' label={[{ type: 'PLAIN_TEXT', value: 'Snapshot link' }]} />); - expect(container).toMatchSnapshot(); - }); + it('renders the expected external link markup', () => { + render(<LinkSpan href='https://example.com' label={[{ type: 'PLAIN_TEXT', value: 'Snapshot link' }]} />); + expect(screen.getByRole('link')).toMatchInlineSnapshot(` +<a + href="https://example.com/" + rel="noopener noreferrer" + target="_blank" +> + Snapshot link +</a> +`); + }); - it('matches snapshot for internal link', () => { - const { container } = render( - <LinkSpan href='http://localhost:3000/channel/test' label={[{ type: 'PLAIN_TEXT', value: 'Internal' }]} />, - ); - expect(container).toMatchSnapshot(); - }); + it('renders the expected internal link markup', () => { + render(<LinkSpan href='http://localhost:3000/channel/test' label={[{ type: 'PLAIN_TEXT', value: 'Internal' }]} />); + expect(screen.getByRole('link')).toMatchInlineSnapshot(` +<a + href="http://localhost:3000/channel/test" +> + Internal +</a> +`); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/elements/LinkSpan.spec.tsx` around lines 65 - 75, The tests in LinkSpan.spec.tsx use container-level snapshots that are brittle; either narrow the snapshot to the anchor element produced by the LinkSpan component or remove the container snapshots and replace them with focused assertions (e.g., check that the rendered anchor has correct href, rel/target for external links, and pathname for internal links). Update the two tests that call render(...LinkSpan...) and expect(container).toMatchSnapshot() to instead query for the anchor node (e.g., using container.querySelector('a') or getByRole('link')) and then either snapshot that anchor element only or assert its attributes/text content; keep tests for both external and internal behaviors in LinkSpan.spec.tsx.packages/gazzodown/src/code/CodeBlock.spec.tsx (1)
77-87: Consider restoringnavigator.clipboardafter test to prevent pollution.The clipboard mock mutates the global
navigatorobject but doesn't restore it afterward. While Jest typically isolates test files, this could cause issues if other tests in the same file depend on the original clipboard behavior.♻️ Suggested cleanup pattern
it('copies code to clipboard on copy button click', async () => { + const originalClipboard = navigator.clipboard; const writeText = jest.fn().mockResolvedValue(undefined); Object.assign(navigator, { clipboard: { writeText } }); + + try { renderCodeBlock({ lines: codeLines(['const a = 1;', 'const b = 2;']) }); const button = await screen.findByTitle('Copy'); await userEvent.click(button); expect(writeText).toHaveBeenCalledWith('const a = 1;\nconst b = 2;'); + } finally { + Object.assign(navigator, { clipboard: originalClipboard }); + } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/code/CodeBlock.spec.tsx` around lines 77 - 87, The test "copies code to clipboard on copy button click" mutates global navigator.clipboard (writeText) and should restore it after running; update the test (or add an afterEach) to store the original navigator.clipboard before Object.assign, set the mock (writeText) for navigator.clipboard, and then restore the original clipboard object after the test completes so renderCodeBlock, userEvent.click, and assertions use the mock but other tests are unaffected.packages/gazzodown/src/elements/Timestamp/RelativeTime.tsx (1)
10-25: Prefer self-documenting thresholds over inline implementation comments.These comments just restate the branches. Named duration constants would make the function readable without carrying comment noise. As per coding guidelines, "Avoid code comments in the implementation".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/elements/Timestamp/RelativeTime.tsx` around lines 10 - 25, Replace the inline numeric literals and explanatory comments in RelativeTime.tsx with self-documenting named duration constants (e.g., ONE_SECOND_MS = 1000, ONE_MINUTE_MS = 60000, ONE_HOUR_MS = 3600000, ONE_DAY_MS = 86400000) and then use those constants in the conditional thresholds that reference timeToRefresh and in the returned refresh intervals (return ONE_SECOND_MS, return ONE_MINUTE_MS / 2, return ONE_MINUTE_MS * 5, return ONE_HOUR_MS). Remove the comment lines that merely restate logic so the branch intent is clear from the constant names; keep all logic in the same function (where timeToRefresh is evaluated) and ensure constants are declared near the top of the file or module scope for reuse.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/gazzodown/src/code/CodeBlock.spec.tsx`:
- Around line 47-68: The tests use screen.findByRole('code') but the CodeBlock
component renders a <code> element without role, so update either the component
or the tests: in CodeBlock (component name CodeBlock in CodeBlock.tsx) add
role="code" to the <code> element so findByRole('code') works, or modify the
spec (CodeBlock.spec.tsx) to query the element differently (e.g., use
screen.getByText / getByTestId or add a data-testid on the <code> element) and
update the three tests that call findByRole('code') accordingly.
In `@packages/gazzodown/src/elements/Timestamp/RelativeTime.spec.tsx`:
- Around line 63-70: The snapshot is brittle due to locale/timezone variations
in the <RelativeTime> rendering; update the test in RelativeTime.spec.tsx to
remove the snapshot and instead query the rendered <time> element (e.g. via
container.querySelector('time') or screen.getByRole('time'/'time' selector) and
assert its dateTime attribute equals the original value.toISOString() and that
its visible text matches the expected relative string (e.g. "in X days" or the
specific relative output your component produces given language 'en'); keep the
MarkupInteractionContext.Provider and the same value Date object but replace
expect(container).toMatchSnapshot() with these deterministic assertions against
dateTime and textContent.
---
Duplicate comments:
In `@packages/gazzodown/src/blocks/SpoilerBlock.spec.tsx`:
- Around line 30-36: The test "reveals content on Space key" in
SpoilerBlock.spec.tsx currently only asserts the trigger button disappears;
extend it to also assert the revealed content is rendered by checking for the
spoiler text (e.g., "Secret") after simulating the Space key. After calling
userEvent.keyboard(' '), add an assertion (using screen.getByText or
screen.findByText) that the paragraph text from the rendered SpoilerBlock is
present, while keeping the existing check that the button is not in the
document.
In `@packages/gazzodown/src/blocks/TaskListBlock.spec.tsx`:
- Around line 23-32: The test currently only verifies that the onTaskChecked
factory is accessed during render; update TaskListBlock.spec.tsx to simulate a
user toggling the checkbox so the returned handler is executed: mock
onTaskChecked as jest.fn().mockReturnValue(jest.fn()) (or capture the returned
handler with a second mock), render TaskListBlock inside
MarkupInteractionContext.Provider, locate the checkbox (e.g.,
getByRole('checkbox') or getByLabelText) and fire a click (userEvent.click or
fireEvent.click), then assert onTaskChecked was called with the task object and
that the returned handler mock was invoked.
In `@packages/gazzodown/src/elements/Timestamp/RelativeTime.spec.tsx`:
- Around line 49-61: The test for RelativeTime doesn't verify that the interval
actually changed React state; wrap jest.advanceTimersByTime(2000) in React's
act() (or the testing-library act helper) to flush effects, then assert that
container.textContent !== initialText (i.e., the post-tick text differs from the
saved initialText) to prove the component updated; locate the test case using
the RelativeTime component and the initialText variable to apply this change.
In `@packages/gazzodown/src/elements/Timestamp/RelativeTime.tsx`:
- Around line 7-27: getTimeToRefresh currently uses a signed delta (time -
Date.now()), so past timestamps yield negative values and always hit the "<
60000" branch; change it to use the absolute difference (e.g., Math.abs(time -
Date.now())) before evaluating thresholds so past times fall into the proper
refresh buckets; update the logic inside getTimeToRefresh in RelativeTime.tsx
accordingly and keep the same return values for each threshold.
- Around line 37-47: The effect hardcodes locale to 'en' and misses language in
its deps, and it uses the stale local variable time when computing the next
refresh; update the effect to call intlFormatDistance(value.getTime(),
Date.now(), { locale: language ?? 'en' }) and
setTimeToRefresh(getTimeToRefresh(value)) inside the interval, and add language
to the useEffect dependency array (keep value, language, and timeToRefresh as
needed) so locale changes trigger correct refreshes; verify initial useState
calls (text/setTime and timeToRefresh/setTimeToRefresh) continue to use language
?? 'en' and getTimeToRefresh(value) consistently.
In `@packages/gazzodown/src/elements/Timestamp/Timestamp.spec.tsx`:
- Around line 56-67: The test uses jest.useFakeTimers()/jest.setSystemTime
around rendering Timestamp (render(<Timestamp format='R' value={date} />)) but
restores timers with jest.useRealTimers() only at the end, so a failing
assertion can leak fake timers; wrap the fake-timer block in a try/finally (call
jest.useRealTimers() in finally) or move timer cleanup to an afterEach that
always calls jest.useRealTimers() to guarantee restoration even on assertion
failure.
- Around line 76-79: The test for Timestamp('f') should avoid
environment-dependent snapshotting of host-localized markup produced by Time
(which uses new Date(...).toLocaleString()); update the test in
Timestamp.spec.tsx to stop using toMatchSnapshot and instead assert
deterministic pieces: render <Timestamp format='f' value={date} />, find the
Time/Timestamp element (by role/testid/text), assert its dateTime attribute
equals the original ISO value and assert the visible text/format is as expected
(or mock Intl/Date.prototype.toLocaleString if you need a specific string);
reference the Timestamp component and the Time-rendered title to locate where to
change the test.
In `@packages/gazzodown/src/Markup.spec.tsx`:
- Around line 285-291: The test currently mutates the live render tree by
calling container.querySelector('math')?.remove() before snapshotting; instead,
create a deep clone of the rendered container (use container.cloneNode(true)),
remove or sanitize the <math> node(s) from that clone (e.g.,
clonedContainer.querySelector('math')?.remove()), and call
expect(clonedContainer).toMatchSnapshot() so the live DOM (and subsequent
assertions like screen.getByRole) remain unchanged; update the test around
container.querySelector('math')?.remove() to operate on the cloned node.
---
Nitpick comments:
In `@packages/gazzodown/src/code/CodeBlock.spec.tsx`:
- Around line 77-87: The test "copies code to clipboard on copy button click"
mutates global navigator.clipboard (writeText) and should restore it after
running; update the test (or add an afterEach) to store the original
navigator.clipboard before Object.assign, set the mock (writeText) for
navigator.clipboard, and then restore the original clipboard object after the
test completes so renderCodeBlock, userEvent.click, and assertions use the mock
but other tests are unaffected.
In `@packages/gazzodown/src/elements/InlineElements.spec.tsx`:
- Around line 31-134: Tests are missing coverage for SPOILER, INLINE_KATEX, and
TIMESTAMP inline element cases; add unit tests in InlineElements.spec.tsx that
exercise InlineElements (or the existing helper renderInlineElements) with: a
SPOILER element verifying it renders a SpoilerSpan; an INLINE_KATEX element
verifying it renders a KatexElement wrapped by a KatexErrorBoundary (use
Suspense/fallback if needed); and a TIMESTAMP element verifying it renders a
Timestamp component; keep tests consistent with existing patterns (use screen
queries like getByText/getByRole or closest checks) and mirror how other cases
(e.g., BOLD, EMOJI) are asserted.
In `@packages/gazzodown/src/elements/LinkSpan.spec.tsx`:
- Around line 65-75: The tests in LinkSpan.spec.tsx use container-level
snapshots that are brittle; either narrow the snapshot to the anchor element
produced by the LinkSpan component or remove the container snapshots and replace
them with focused assertions (e.g., check that the rendered anchor has correct
href, rel/target for external links, and pathname for internal links). Update
the two tests that call render(...LinkSpan...) and
expect(container).toMatchSnapshot() to instead query for the anchor node (e.g.,
using container.querySelector('a') or getByRole('link')) and then either
snapshot that anchor element only or assert its attributes/text content; keep
tests for both external and internal behaviors in LinkSpan.spec.tsx.
In `@packages/gazzodown/src/elements/PreviewInlineElements.spec.tsx`:
- Around line 6-124: Add a unit test for the INLINE_KATEX branch in
PreviewInlineElements: render <PreviewInlineElements> with an element of type
'INLINE_KATEX' (value e.g. 'x^2') and assert the expected KaTeX output appears;
also add a separate test that simulates a KaTeX render error (e.g. by mocking
PreviewKatexElement or forcing KatexErrorBoundary to throw) and assert the error
boundary fallback is shown. Target the PreviewInlineElements spec and reference
the INLINE_KATEX token, KatexErrorBoundary and PreviewKatexElement when locating
the implementation to cover both success and error cases.
In `@packages/gazzodown/src/elements/Timestamp/RelativeTime.tsx`:
- Around line 10-25: Replace the inline numeric literals and explanatory
comments in RelativeTime.tsx with self-documenting named duration constants
(e.g., ONE_SECOND_MS = 1000, ONE_MINUTE_MS = 60000, ONE_HOUR_MS = 3600000,
ONE_DAY_MS = 86400000) and then use those constants in the conditional
thresholds that reference timeToRefresh and in the returned refresh intervals
(return ONE_SECOND_MS, return ONE_MINUTE_MS / 2, return ONE_MINUTE_MS * 5,
return ONE_HOUR_MS). Remove the comment lines that merely restate logic so the
branch intent is clear from the constant names; keep all logic in the same
function (where timeToRefresh is evaluated) and ensure constants are declared
near the top of the file or module scope for reuse.
In `@packages/gazzodown/src/mentions/PreviewChannelMentionElement.spec.tsx`:
- Around line 6-14: The two tests for PreviewChannelMentionElement both assert
the same behavior; replace them with a parameterized test using Jest's it.each
to iterate over channel names (e.g., 'general', 'random-channel') and assert
container has text '#<name>'; update the spec to remove the duplicate it blocks
and add a single it.each that renders PreviewChannelMentionElement with the
parameterized mention prop and checks toHaveTextContent(`#${mention}`).
In `@packages/gazzodown/src/mentions/PreviewUserMentionElement.spec.tsx`:
- Around line 6-14: Consolidate the two duplicate tests in
PreviewUserMentionElement.spec.tsx by replacing the separate it blocks with a
parameterized test (e.g., using it.each) that iterates over usernames and
asserts the component renders the '@' prefixed value; update tests referencing
PreviewUserMentionElement and the render +
expect(container).toHaveTextContent('@{username}') pattern so a single table
drives cases like 'john.doe' and 'admin'.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 35926421-d5de-44f6-8f07-7f4f6a158f6c
⛔ Files ignored due to path filters (44)
packages/gazzodown/src/__snapshots__/Markup.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/__snapshots__/PreviewMarkup.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/blocks/__snapshots__/HeadingBlock.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/blocks/__snapshots__/OrderedListBlock.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/blocks/__snapshots__/ParagraphBlock.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/blocks/__snapshots__/QuoteBlock.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/blocks/__snapshots__/SpoilerBlock.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/blocks/__snapshots__/TaskListBlock.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/blocks/__snapshots__/UnorderedListBlock.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/code/__snapshots__/CodeBlock.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/code/__snapshots__/CodeElement.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/code/__snapshots__/PreviewCodeBlock.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/code/__snapshots__/PreviewCodeElement.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/colors/__snapshots__/ColorElement.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/colors/__snapshots__/PreviewColorElement.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/elements/Timestamp/__snapshots__/DateTimeFormats.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/elements/Timestamp/__snapshots__/RelativeTime.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/elements/Timestamp/__snapshots__/Time.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/elements/Timestamp/__snapshots__/Timestamp.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/elements/Timestamp/__snapshots__/TimestampWrapper.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/elements/__snapshots__/BoldSpan.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/elements/__snapshots__/ImageElement.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/elements/__snapshots__/InlineElements.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/elements/__snapshots__/ItalicSpan.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/elements/__snapshots__/LinkSpan.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/elements/__snapshots__/PlainSpan.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/elements/__snapshots__/PreviewInlineElements.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/elements/__snapshots__/SpoilerSpan.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/elements/__snapshots__/StrikeSpan.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/emoji/__snapshots__/BigEmojiBlock.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/emoji/__snapshots__/BigEmojiElement.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/emoji/__snapshots__/Emoji.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/emoji/__snapshots__/EmojiRenderer.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/emoji/__snapshots__/PreviewBigEmojiBlock.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/emoji/__snapshots__/PreviewEmojiElement.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/katex/__snapshots__/KatexBlock.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/katex/__snapshots__/KatexElement.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/katex/__snapshots__/KatexErrorBoundary.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/katex/__snapshots__/PreviewKatexBlock.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/katex/__snapshots__/PreviewKatexElement.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/mentions/__snapshots__/ChannelMentionElement.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/mentions/__snapshots__/PreviewChannelMentionElement.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/mentions/__snapshots__/PreviewUserMentionElement.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/mentions/__snapshots__/UserMentionElement.spec.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (57)
packages/gazzodown/src/Markup.spec.tsxpackages/gazzodown/src/Markup.stories.tsxpackages/gazzodown/src/PreviewMarkup.spec.tsxpackages/gazzodown/src/blocks/HeadingBlock.spec.tsxpackages/gazzodown/src/blocks/OrderedListBlock.spec.tsxpackages/gazzodown/src/blocks/ParagraphBlock.spec.tsxpackages/gazzodown/src/blocks/QuoteBlock.spec.tsxpackages/gazzodown/src/blocks/SpoilerBlock.spec.tsxpackages/gazzodown/src/blocks/TaskListBlock.spec.tsxpackages/gazzodown/src/blocks/UnorderedListBlock.spec.tsxpackages/gazzodown/src/code/CodeBlock.spec.tsxpackages/gazzodown/src/code/CodeElement.spec.tsxpackages/gazzodown/src/code/PreviewCodeBlock.spec.tsxpackages/gazzodown/src/code/PreviewCodeElement.spec.tsxpackages/gazzodown/src/colors/ColorElement.spec.tsxpackages/gazzodown/src/colors/PreviewColorElement.spec.tsxpackages/gazzodown/src/elements/BoldSpan.spec.tsxpackages/gazzodown/src/elements/ImageElement.spec.tsxpackages/gazzodown/src/elements/InlineElements.spec.tsxpackages/gazzodown/src/elements/ItalicSpan.spec.tsxpackages/gazzodown/src/elements/LinkSpan.spec.tsxpackages/gazzodown/src/elements/PlainSpan.spec.tsxpackages/gazzodown/src/elements/PreviewInlineElements.spec.tsxpackages/gazzodown/src/elements/SpoilerSpan.spec.tsxpackages/gazzodown/src/elements/StrikeSpan.spec.tsxpackages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsxpackages/gazzodown/src/elements/Timestamp/FullDate.tsxpackages/gazzodown/src/elements/Timestamp/FullDateLong.tsxpackages/gazzodown/src/elements/Timestamp/LongDate.tsxpackages/gazzodown/src/elements/Timestamp/LongTime.tsxpackages/gazzodown/src/elements/Timestamp/RelativeTime.spec.tsxpackages/gazzodown/src/elements/Timestamp/RelativeTime.tsxpackages/gazzodown/src/elements/Timestamp/ShortDate.tsxpackages/gazzodown/src/elements/Timestamp/ShortTime.tsxpackages/gazzodown/src/elements/Timestamp/Time.spec.tsxpackages/gazzodown/src/elements/Timestamp/Time.tsxpackages/gazzodown/src/elements/Timestamp/Timestamp.spec.tsxpackages/gazzodown/src/elements/Timestamp/Timestamp.tsxpackages/gazzodown/src/elements/Timestamp/TimestampWrapper.spec.tsxpackages/gazzodown/src/elements/Timestamp/TimestampWrapper.tsxpackages/gazzodown/src/elements/Timestamp/index.tspackages/gazzodown/src/elements/Timestamp/index.tsxpackages/gazzodown/src/emoji/BigEmojiBlock.spec.tsxpackages/gazzodown/src/emoji/BigEmojiElement.spec.tsxpackages/gazzodown/src/emoji/Emoji.spec.tsxpackages/gazzodown/src/emoji/EmojiRenderer.spec.tsxpackages/gazzodown/src/emoji/PreviewBigEmojiBlock.spec.tsxpackages/gazzodown/src/emoji/PreviewEmojiElement.spec.tsxpackages/gazzodown/src/katex/KatexBlock.spec.tsxpackages/gazzodown/src/katex/KatexElement.spec.tsxpackages/gazzodown/src/katex/KatexErrorBoundary.spec.tsxpackages/gazzodown/src/katex/PreviewKatexBlock.spec.tsxpackages/gazzodown/src/katex/PreviewKatexElement.spec.tsxpackages/gazzodown/src/mentions/ChannelMentionElement.spec.tsxpackages/gazzodown/src/mentions/PreviewChannelMentionElement.spec.tsxpackages/gazzodown/src/mentions/PreviewUserMentionElement.spec.tsxpackages/gazzodown/src/mentions/UserMentionElement.spec.tsx
💤 Files with no reviewable changes (1)
- packages/gazzodown/src/elements/Timestamp/index.tsx
🚧 Files skipped from review as they are similar to previous changes (33)
- packages/gazzodown/src/Markup.stories.tsx
- packages/gazzodown/src/elements/Timestamp/Time.tsx
- packages/gazzodown/src/elements/Timestamp/LongDate.tsx
- packages/gazzodown/src/elements/SpoilerSpan.spec.tsx
- packages/gazzodown/src/elements/Timestamp/Time.spec.tsx
- packages/gazzodown/src/code/PreviewCodeBlock.spec.tsx
- packages/gazzodown/src/elements/Timestamp/FullDate.tsx
- packages/gazzodown/src/katex/KatexElement.spec.tsx
- packages/gazzodown/src/elements/Timestamp/TimestampWrapper.spec.tsx
- packages/gazzodown/src/katex/KatexBlock.spec.tsx
- packages/gazzodown/src/mentions/UserMentionElement.spec.tsx
- packages/gazzodown/src/mentions/ChannelMentionElement.spec.tsx
- packages/gazzodown/src/katex/KatexErrorBoundary.spec.tsx
- packages/gazzodown/src/emoji/PreviewEmojiElement.spec.tsx
- packages/gazzodown/src/blocks/OrderedListBlock.spec.tsx
- packages/gazzodown/src/elements/Timestamp/TimestampWrapper.tsx
- packages/gazzodown/src/colors/ColorElement.spec.tsx
- packages/gazzodown/src/emoji/BigEmojiBlock.spec.tsx
- packages/gazzodown/src/elements/PlainSpan.spec.tsx
- packages/gazzodown/src/elements/Timestamp/index.ts
- packages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsx
- packages/gazzodown/src/code/CodeElement.spec.tsx
- packages/gazzodown/src/katex/PreviewKatexBlock.spec.tsx
- packages/gazzodown/src/PreviewMarkup.spec.tsx
- packages/gazzodown/src/elements/Timestamp/ShortDate.tsx
- packages/gazzodown/src/elements/Timestamp/Timestamp.tsx
- packages/gazzodown/src/code/PreviewCodeElement.spec.tsx
- packages/gazzodown/src/blocks/HeadingBlock.spec.tsx
- packages/gazzodown/src/emoji/EmojiRenderer.spec.tsx
- packages/gazzodown/src/elements/ItalicSpan.spec.tsx
- packages/gazzodown/src/blocks/ParagraphBlock.spec.tsx
- packages/gazzodown/src/elements/Timestamp/ShortTime.tsx
- packages/gazzodown/src/elements/ImageElement.spec.tsx
Comment on lines
+47
to
+68
| it('applies language class when language is provided', async () => { | ||
| renderCodeBlock({ lines: codeLines(['x = 1']), language: 'python' }); | ||
|
|
||
| const code = await screen.findByRole('code'); | ||
| expect(code).toHaveClass('language-python'); | ||
| }); | ||
|
|
||
| it('applies code-colors class without language class when language is undefined', async () => { | ||
| renderCodeBlock({ lines: codeLines(['x = 1']) }); | ||
|
|
||
| const code = await screen.findByRole('code'); | ||
| expect(code).toHaveClass('code-colors'); | ||
| expect(code.className).not.toContain('language-'); | ||
| }); | ||
|
|
||
| it('applies code-colors class when language is "none"', async () => { | ||
| renderCodeBlock({ lines: codeLines(['x = 1']), language: 'none' }); | ||
|
|
||
| const code = await screen.findByRole('code'); | ||
| expect(code).toHaveClass('code-colors'); | ||
| expect(code.className).not.toContain('language-'); | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, search for role="code" in the CodeBlock component file
echo "=== Searching for role='code' in CodeBlock.tsx ==="
rg -n 'role.*=.*["\x27]code["\x27]' packages/gazzodown/src/code/
echo ""
echo "=== Searching for CodeBlock.tsx implementation ==="
fd CodeBlock.tsx packages/gazzodown/src/code/Repository: RocketChat/Rocket.Chat
Length of output: 259
🏁 Script executed:
#!/bin/bash
# Find and examine the CodeBlock.tsx file
find packages/gazzodown/src/code -name "CodeBlock.tsx" -type f -exec cat -n {} \;Repository: RocketChat/Rocket.Chat
Length of output: 3684
🏁 Script executed:
#!/bin/bash
# Let's also check the test file to understand the component structure better
cat -n packages/gazzodown/src/code/CodeBlock.spec.tsx | head -80Repository: RocketChat/Rocket.Chat
Length of output: 3261
Fix the findByRole('code') queries—the <code> element has no role="code" attribute.
The CodeBlock component renders the <code> element without an explicit role attribute (line 101 of CodeBlock.tsx). Since <code> has no implicit ARIA role and "code" is not a standard ARIA role, screen.findByRole('code') will fail to locate the element at runtime. Add role="code" to the <code> element or use an alternative query method (e.g., getByText or data-testid).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/gazzodown/src/code/CodeBlock.spec.tsx` around lines 47 - 68, The
tests use screen.findByRole('code') but the CodeBlock component renders a <code>
element without role, so update either the component or the tests: in CodeBlock
(component name CodeBlock in CodeBlock.tsx) add role="code" to the <code>
element so findByRole('code') works, or modify the spec (CodeBlock.spec.tsx) to
query the element differently (e.g., use screen.getByText / getByTestId or add a
data-testid on the <code> element) and update the three tests that call
findByRole('code') accordingly.
Member
Author
There was a problem hiding this comment.
The <code> element has an implicit ARIA role of code per WAI-ARIA 1.3, which testing-library/jsdom already supports. The screen.findByRole('code') calls work correctly, as confirmed by all 268 tests passing. No fix needed.
Contributor
There was a problem hiding this comment.
@tassoevan, you're right — I apologize for the incorrect flag. WAI-ARIA 1.3 introduced the code role as an implicit ARIA role for the <code> element, and Testing Library's getByRole queries respect default/implicit roles (e.g., <button /> has the button role without explicitly setting the role attribute). The empirical confirmation from all 268 tests passing makes this clear. My earlier analysis was based on WAI-ARIA 1.2 behavior where <code> had no implicit role.
✏️ Learnings added
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/code/CodeBlock.spec.tsx:47-68
Timestamp: 2026-03-06T18:10:15.268Z
Learning: In the RocketChat/Rocket.Chat `packages/gazzodown` package and more broadly, the HTML `<code>` element has an implicit ARIA role of `code` per WAI-ARIA 1.3, and `testing-library/dom` / jsdom supports it. Therefore, `screen.getByRole('code')` / `screen.findByRole('code')` correctly locates `<code>` elements without needing an explicit `role="code"` attribute. Do NOT flag `findByRole('code')` as invalid in future reviews.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `expect` matchers for assertions (`toEqual`, `toContain`, `toBeTruthy`, `toHaveLength`, etc.) instead of `assert` statements in Playwright tests
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to **/*.{ts,tsx,js} : Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Prefer web-first assertions (`toBeVisible`, `toHaveText`, etc.) in Playwright tests
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Maintain test isolation between test cases in Playwright tests
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/page-objects/fragments/home-content.ts:60-82
Timestamp: 2026-02-24T19:36:55.089Z
Learning: In RocketChat/Rocket.Chat e2e tests (apps/meteor/tests/e2e/page-objects/fragments/home-content.ts), thread message preview listitems do not have aria-roledescription="message", so lastThreadMessagePreview locator cannot be scoped to messageListItems (which filters for aria-roledescription="message"). It should remain scoped to page.getByRole('listitem') or mainMessageList.getByRole('listitem').
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/omnichannel/omnichannel-send-pdf-transcript.spec.ts:66-67
Timestamp: 2026-02-24T19:22:52.984Z
Learning: In RocketChat/Rocket.Chat Playwright e2e tests, prefer using translated text and ARIA roles (getByRole, getByText, etc.) over data-qa locators. If translation values change, update the corresponding test locators accordingly. Never prefer data-qa locators.
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/page-objects/fragments/message.ts:7-7
Timestamp: 2026-02-24T19:39:52.176Z
Learning: In RocketChat/Rocket.Chat e2e tests, never prefer data-qa attributes to locate elements. Always prefer semantic locators (getByRole, getByLabel, getByText, getByTitle) and ARIA-based selectors over data-qa locators.
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 36987
File: apps/meteor/tests/e2e/page-objects/fragments/room-toolbar.ts:10-20
Timestamp: 2025-09-23T19:22:59.217Z
Learning: In Playwright e2e tests, prefer stable selectors like data-qa-id attributes over localized text in getByRole() or getByText() calls to prevent test failures when UI language changes. Test translations separately by validating actual text content after ensuring UI interactions work with stable selectors.
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Avoid using `page.locator()` in Playwright tests - always prefer semantic locators such as `page.getByRole()`, `page.getByLabel()`, `page.getByText()`, or `page.getByTitle()`
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37091
File: ee/packages/abac/jest.config.ts:4-7
Timestamp: 2025-12-10T21:00:54.909Z
Learning: Rocket.Chat monorepo: Jest testMatch pattern '<rootDir>/src/**/*.spec.(ts|js|mjs)' is valid in this repo and used across multiple packages (e.g., packages/tools, ee/packages/omnichannel-services). Do not flag it as invalid in future reviews.
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37408
File: apps/meteor/client/views/admin/ABAC/useRoomAttributeOptions.tsx:53-69
Timestamp: 2025-11-10T19:06:20.146Z
Learning: In the Rocket.Chat repository, do not provide suggestions or recommendations about code sections marked with TODO comments. The maintainers have already identified these as future work and external reviewers lack the full context about implementation plans and timing.
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsx:20-23
Timestamp: 2026-03-06T18:09:14.766Z
Learning: In the RocketChat/Rocket.Chat gazzodown package (`packages/gazzodown`), tests are intended to run under the UTC timezone, but as of PR `#39397` this is NOT yet explicitly enforced in `jest.config.ts` or the `package.json` test scripts (which just run `jest` without `TZ=UTC`). To make timezone-sensitive snapshot tests reliable across all environments, `TZ=UTC` should be added to the test scripts in `package.json` or to `jest.config.ts` via `testEnvironmentOptions.timezone`. Without explicit UTC enforcement, snapshot tests involving date-fns formatted output or `toLocaleString()` will fail for contributors in non-UTC timezones.
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (`test`, `page`, `expect`) for consistency in test files
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Group related tests in the same file
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to **/*.spec.ts : Use descriptive test names that clearly communicate expected behavior in Playwright tests
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `test.step()` for complex test scenarios to improve organization in Playwright tests
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/elements/Timestamp/RelativeTime.spec.tsx:63-70
Timestamp: 2026-03-06T18:02:11.732Z
Learning: In Rocket.Chat, tests within the gazzodown package (and the broader test suite) should run in UTC to make locale/timezone-sensitive outputs from toLocaleString() deterministic. Ensure snapshot tests that capture values like title attributes based on toLocaleString() remain stable by running tests in UTC. Actions you can take: - configure the test environment to TZ=UTC (e.g., in CI or local npm/yarn test scripts), - optionally set process.env.TZ = 'UTC' in Jest/setupFiles or the test bootstrap, - avoid asserting locale-sensitive strings directly; prefer targeted assertions or mocks where necessary. Apply this guideline to all spec files under packages/gazzodown/src that involve timestamp rendering (pattern above).
91e646d to
1d4ce5d
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (5)
packages/gazzodown/src/blocks/SpoilerBlock.spec.tsx (1)
30-36:⚠️ Potential issue | 🟡 MinorAssert the revealed content after Space activation.
This still false-passes if the button disappears for any reason other than a successful reveal. Add a content assertion after the keypress as well.
Suggested patch
it('reveals content on Space key', async () => { render(<SpoilerBlock>{[{ type: 'PARAGRAPH', value: [{ type: 'PLAIN_TEXT', value: 'Secret' }] }]}</SpoilerBlock>); screen.getByRole('button').focus(); await userEvent.keyboard(' '); expect(screen.queryByRole('button')).not.toBeInTheDocument(); + expect(screen.getByText('Secret')).toBeInTheDocument(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/blocks/SpoilerBlock.spec.tsx` around lines 30 - 36, The test "reveals content on Space key" in SpoilerBlock.spec.tsx currently only asserts the button is gone which can false-pass; after simulating the Space key on the element returned by screen.getByRole('button') in the SpoilerBlock test, add an assertion that the revealed content "Secret" (the PLAIN_TEXT value used in the render) is present (e.g., using screen.getByText or expect(...).toBeInTheDocument()) to ensure the reveal actually occurred for the SpoilerBlock component.packages/gazzodown/src/code/PreviewCodeElement.spec.tsx (1)
11-13:⚠️ Potential issue | 🟡 MinorAssert the empty-state output, not just successful render.
containerexists wheneverrender()succeeds, so this will not catch fallback text or unexpected markup forcode=''. Assert the rendered output instead.Suggested change
- it('renders empty string without crashing', () => { + it('renders empty string without fallback content', () => { const { container } = render(<PreviewCodeElement code='' />); - expect(container).toBeInTheDocument(); + expect(container.textContent).toBe(''); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/code/PreviewCodeElement.spec.tsx` around lines 11 - 13, The test "renders empty string without crashing" only asserts that render() succeeded by checking container; update it to assert the actual empty-state output of the PreviewCodeElement when code='' instead: locate the test for PreviewCodeElement (the it(...) block) and replace the container existence assertion with a specific assertion that the fallback text/markup is present (e.g., using screen.getByText(...) or container.querySelector(...) to match the expected empty-state string or element), or alternatively assert the rendered markup via a snapshot to ensure the component shows the intended empty-state output rather than just rendering.packages/gazzodown/src/elements/Timestamp/LongDate.tsx (1)
9-9:⚠️ Potential issue | 🟠 MajorUse a date-only token in
LongDate.Line 9 formats with
'Pp', which includes time, so theDbranch renders more than a long date. This should use'PP'instead.Proposed fix
-const LongDate = ({ value }: LongDateProps) => <Time value={format(value, 'Pp')} dateTime={value.toISOString()} />; +const LongDate = ({ value }: LongDateProps) => <Time value={format(value, 'PP')} dateTime={value.toISOString()} />;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/elements/Timestamp/LongDate.tsx` at line 9, The LongDate component currently formats the date using the 'Pp' token (which includes time) causing the D branch to render time as well; update the format call in LongDate (component LongDate, prop value) to use the date-only token 'PP' and keep dateTime as value.toISOString() so the rendered output is a long date without the time portion.packages/gazzodown/src/elements/Timestamp/RelativeTime.tsx (2)
36-47:⚠️ Potential issue | 🟠 MajorKeep interval updates in the active locale.
Line 42 hardcodes
'en', and Line 47 omitslanguage, so the label can flip to English after the first tick and never react to locale changes.Proposed fix
const time = value.getTime(); const { language } = useContext(MarkupInteractionContext); - const [text, setTime] = useState(() => intlFormatDistance(time, Date.now(), { locale: language ?? 'en' })); + const locale = language ?? 'en'; + const [text, setTime] = useState(() => intlFormatDistance(time, Date.now(), { locale })); const [timeToRefresh, setTimeToRefresh] = useState(() => getTimeToRefresh(time)); useEffect(() => { const interval = setInterval(() => { - setTime(intlFormatDistance(value.getTime(), Date.now(), { locale: 'en' })); + setTime(intlFormatDistance(time, Date.now(), { locale })); setTimeToRefresh(getTimeToRefresh(time)); }, timeToRefresh); return () => clearInterval(interval); - }, [time, timeToRefresh, value]); + }, [locale, time, timeToRefresh]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/elements/Timestamp/RelativeTime.tsx` around lines 36 - 47, The effect currently hardcodes 'en' and uses stale state when scheduling ticks, causing locale flips and missed locale updates; update both intlFormatDistance calls to use the active locale from useContext(MarkupInteractionContext) (language ?? 'en') instead of the literal 'en', call getTimeToRefresh with the actual value (e.g., value or value.getTime()) rather than the stale time state, and add language to the useEffect dependency list so ticks re-run when locale changes (refer to MarkupInteractionContext, intlFormatDistance, getTimeToRefresh, the text/setTime and timeToRefresh state variables, and the useEffect that currently depends on [time, timeToRefresh, value]).
7-27:⚠️ Potential issue | 🟠 MajorUse the absolute distance when choosing the refresh cadence.
For past timestamps, Line 8 becomes negative, so this function returns
1000forever. That keeps old messages re-rendering every second instead of backing off with age.Proposed fix
const getTimeToRefresh = (time: number): number => { - const timeToRefresh = time - Date.now(); + const timeToRefresh = Math.abs(time - Date.now());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/elements/Timestamp/RelativeTime.tsx` around lines 7 - 27, getTimeToRefresh currently uses time - Date.now(), so past timestamps produce negative values and always return 1000ms; change it to use the absolute distance (e.g. Math.abs(time - Date.now())) before comparing to thresholds in getTimeToRefresh so older timestamps back off to minute/hour intervals instead of refreshing every second; update the comparisons/return values accordingly while keeping the same threshold logic for seconds/minutes/hours ranges.
🧹 Nitpick comments (6)
packages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsx (1)
15-17: Consider extracting the repeated DOM query pattern into a helper.The same
document.querySelector('time')pattern with eslint-disable appears 6 times. A small helper would reduce repetition and centralize the eslint suppression.♻️ Optional: Extract helper to reduce repetition
+const getTimeElement = () => document.querySelector('time'); + describe('ShortTime', () => { it('renders short time format', () => { const date = new Date('2025-06-15T14:30:00.000Z'); render(<ShortTime value={date} />); - // eslint-disable-next-line testing-library/no-node-access - const timeEl = document.querySelector('time'); + const timeEl = getTimeElement(); expect(timeEl).toHaveAttribute('dateTime', date.toISOString()); });Then add a single eslint-disable at the helper definition and reuse
getTimeElement()across all tests.Also applies to: 31-33, 47-49, 63-65, 79-81, 95-97
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsx` around lines 15 - 17, Tests in DateTimeFormats.spec.tsx repeatedly query the time element with "document.querySelector('time')" and duplicate the eslint-disable comment; extract this into a single helper function (e.g., getTimeElement()) used by all specs, place one eslint-disable-next-line testing-library/no-node-access above the helper definition, and replace the six inline queries in the tests with calls to getTimeElement() so the suppression and DOM access are centralized and duplication removed.packages/gazzodown/src/katex/PreviewKatexBlock.spec.tsx (1)
16-18: Prefer an explicit raw-output assertion over this snapshot.
PreviewKatexBlockcurrently renders only thecodestring, so this snapshot adds little beyond the text assertions above while staying brittle to incidental DOM changes. A direct assertion keeps the intent and is easier to maintain.♻️ Proposed change
- it('matches snapshot', () => { + it('renders raw LaTeX without generating KaTeX markup', () => { const { container } = render(<PreviewKatexBlock code='\\frac{a}{b}' />); - expect(container).toMatchSnapshot(); + expect(container.innerHTML).toBe('\\frac{a}{b}'); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/katex/PreviewKatexBlock.spec.tsx` around lines 16 - 18, The snapshot test for PreviewKatexBlock is brittle; replace it with an explicit assertion that the rendered output contains the expected raw code string. In the test that currently renders PreviewKatexBlock (component name PreviewKatexBlock), use the render result (container or getByText) to assert that the textContent equals or contains "\\frac{a}{b}" (the code prop) instead of calling toMatchSnapshot, ensuring the test checks the actual output directly.packages/gazzodown/src/mentions/ChannelMentionElement.spec.tsx (1)
60-77: Consider narrowing these snapshots.
containersnapshots here are likely to fail on unrelated DOM changes inside the shared mention/highlight components. Since the behavior is already covered with direct assertions above, a node-scoped snapshot or a few focused assertions would be less fragile.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/mentions/ChannelMentionElement.spec.tsx` around lines 60 - 77, The snapshots in ChannelMentionElement.spec.tsx are too broad (using container) and fragile; replace the full-container snapshots for the resolved and unresolved cases with focused assertions or node-scoped snapshots that target the ChannelMentionElement output only. Update the two tests around ChannelMentionElement and MarkupInteractionContext (the resolved test that uses resolveChannelMention and the unresolved test) to either assert specific text/attributes (e.g., channel display name, fallback text/class) or use container.querySelector to capture the specific element and snapshot that node instead of the whole container.packages/gazzodown/src/mentions/UserMentionElement.spec.tsx (1)
84-102: Scope these snapshots to stable output.Snapshotting the whole container here couples the tests to unrelated
MessageHighlight/translation markup changes, so they will churn even when mention behavior is unchanged. Prefer asserting stable semantics directly, or snapshot only the rendered mention node instead of the entire container.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/mentions/UserMentionElement.spec.tsx` around lines 84 - 102, The snapshots are too broad—change the tests for UserMentionElement to scope assertions to the stable rendered mention node instead of the entire container; when using MarkupInteractionContext with resolveUserMention, query the specific mention element returned by UserMentionElement (e.g., find by role, test id, or text content) and snapshot or assert its inner text/attributes, and for the unresolved case assert the specific mention node's fallback rendering rather than container-level snapshots.packages/gazzodown/src/elements/Timestamp/RelativeTime.tsx (1)
10-25: Drop the inline comments ingetTimeToRefresh.The thresholds are already readable from the code, so these comments are just implementation noise.
As per coding guidelines, "Avoid code comments in implementation".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/elements/Timestamp/RelativeTime.tsx` around lines 10 - 25, Remove the inline explanatory comments inside the getTimeToRefresh implementation in RelativeTime (the block that checks timeToRefresh against 60000, 3600000, 86400000 and returns the respective intervals); simply keep the conditional logic and return values (no inline comments) so the function contains only the readable threshold checks and return statements.packages/gazzodown/src/elements/Timestamp/Timestamp.tsx (1)
16-31: Remove the inlinecasecomments.The component names already make each branch self-explanatory, so these comments just add churn to the implementation.
As per coding guidelines, "Avoid code comments in implementation".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gazzodown/src/elements/Timestamp/Timestamp.tsx` around lines 16 - 31, Remove the inline case comments in the switch that returns the timestamp components; locate the switch handling format characters and remove the trailing comments after each case that reference formats (for example on the branches returning ShortTime, LongTime, ShortDate, LongDate, FullDate, FullDateLong, and RelativeTime) so the code reads simply like: case 't': return <ShortTime value={value} />; etc., keeping component names and behavior unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/gazzodown/src/blocks/SpoilerBlock.spec.tsx`:
- Around line 30-36: The test "reveals content on Space key" in
SpoilerBlock.spec.tsx currently only asserts the button is gone which can
false-pass; after simulating the Space key on the element returned by
screen.getByRole('button') in the SpoilerBlock test, add an assertion that the
revealed content "Secret" (the PLAIN_TEXT value used in the render) is present
(e.g., using screen.getByText or expect(...).toBeInTheDocument()) to ensure the
reveal actually occurred for the SpoilerBlock component.
In `@packages/gazzodown/src/code/PreviewCodeElement.spec.tsx`:
- Around line 11-13: The test "renders empty string without crashing" only
asserts that render() succeeded by checking container; update it to assert the
actual empty-state output of the PreviewCodeElement when code='' instead: locate
the test for PreviewCodeElement (the it(...) block) and replace the container
existence assertion with a specific assertion that the fallback text/markup is
present (e.g., using screen.getByText(...) or container.querySelector(...) to
match the expected empty-state string or element), or alternatively assert the
rendered markup via a snapshot to ensure the component shows the intended
empty-state output rather than just rendering.
In `@packages/gazzodown/src/elements/Timestamp/LongDate.tsx`:
- Line 9: The LongDate component currently formats the date using the 'Pp' token
(which includes time) causing the D branch to render time as well; update the
format call in LongDate (component LongDate, prop value) to use the date-only
token 'PP' and keep dateTime as value.toISOString() so the rendered output is a
long date without the time portion.
In `@packages/gazzodown/src/elements/Timestamp/RelativeTime.tsx`:
- Around line 36-47: The effect currently hardcodes 'en' and uses stale state
when scheduling ticks, causing locale flips and missed locale updates; update
both intlFormatDistance calls to use the active locale from
useContext(MarkupInteractionContext) (language ?? 'en') instead of the literal
'en', call getTimeToRefresh with the actual value (e.g., value or
value.getTime()) rather than the stale time state, and add language to the
useEffect dependency list so ticks re-run when locale changes (refer to
MarkupInteractionContext, intlFormatDistance, getTimeToRefresh, the text/setTime
and timeToRefresh state variables, and the useEffect that currently depends on
[time, timeToRefresh, value]).
- Around line 7-27: getTimeToRefresh currently uses time - Date.now(), so past
timestamps produce negative values and always return 1000ms; change it to use
the absolute distance (e.g. Math.abs(time - Date.now())) before comparing to
thresholds in getTimeToRefresh so older timestamps back off to minute/hour
intervals instead of refreshing every second; update the comparisons/return
values accordingly while keeping the same threshold logic for
seconds/minutes/hours ranges.
---
Nitpick comments:
In `@packages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsx`:
- Around line 15-17: Tests in DateTimeFormats.spec.tsx repeatedly query the time
element with "document.querySelector('time')" and duplicate the eslint-disable
comment; extract this into a single helper function (e.g., getTimeElement())
used by all specs, place one eslint-disable-next-line
testing-library/no-node-access above the helper definition, and replace the six
inline queries in the tests with calls to getTimeElement() so the suppression
and DOM access are centralized and duplication removed.
In `@packages/gazzodown/src/elements/Timestamp/RelativeTime.tsx`:
- Around line 10-25: Remove the inline explanatory comments inside the
getTimeToRefresh implementation in RelativeTime (the block that checks
timeToRefresh against 60000, 3600000, 86400000 and returns the respective
intervals); simply keep the conditional logic and return values (no inline
comments) so the function contains only the readable threshold checks and return
statements.
In `@packages/gazzodown/src/elements/Timestamp/Timestamp.tsx`:
- Around line 16-31: Remove the inline case comments in the switch that returns
the timestamp components; locate the switch handling format characters and
remove the trailing comments after each case that reference formats (for example
on the branches returning ShortTime, LongTime, ShortDate, LongDate, FullDate,
FullDateLong, and RelativeTime) so the code reads simply like: case 't': return
<ShortTime value={value} />; etc., keeping component names and behavior
unchanged.
In `@packages/gazzodown/src/katex/PreviewKatexBlock.spec.tsx`:
- Around line 16-18: The snapshot test for PreviewKatexBlock is brittle; replace
it with an explicit assertion that the rendered output contains the expected raw
code string. In the test that currently renders PreviewKatexBlock (component
name PreviewKatexBlock), use the render result (container or getByText) to
assert that the textContent equals or contains "\\frac{a}{b}" (the code prop)
instead of calling toMatchSnapshot, ensuring the test checks the actual output
directly.
In `@packages/gazzodown/src/mentions/ChannelMentionElement.spec.tsx`:
- Around line 60-77: The snapshots in ChannelMentionElement.spec.tsx are too
broad (using container) and fragile; replace the full-container snapshots for
the resolved and unresolved cases with focused assertions or node-scoped
snapshots that target the ChannelMentionElement output only. Update the two
tests around ChannelMentionElement and MarkupInteractionContext (the resolved
test that uses resolveChannelMention and the unresolved test) to either assert
specific text/attributes (e.g., channel display name, fallback text/class) or
use container.querySelector to capture the specific element and snapshot that
node instead of the whole container.
In `@packages/gazzodown/src/mentions/UserMentionElement.spec.tsx`:
- Around line 84-102: The snapshots are too broad—change the tests for
UserMentionElement to scope assertions to the stable rendered mention node
instead of the entire container; when using MarkupInteractionContext with
resolveUserMention, query the specific mention element returned by
UserMentionElement (e.g., find by role, test id, or text content) and snapshot
or assert its inner text/attributes, and for the unresolved case assert the
specific mention node's fallback rendering rather than container-level
snapshots.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cf0beb12-f677-4a3f-a53e-9352bf9cec30
⛔ Files ignored due to path filters (44)
packages/gazzodown/src/__snapshots__/Markup.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/__snapshots__/PreviewMarkup.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/blocks/__snapshots__/HeadingBlock.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/blocks/__snapshots__/OrderedListBlock.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/blocks/__snapshots__/ParagraphBlock.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/blocks/__snapshots__/QuoteBlock.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/blocks/__snapshots__/SpoilerBlock.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/blocks/__snapshots__/TaskListBlock.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/blocks/__snapshots__/UnorderedListBlock.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/code/__snapshots__/CodeBlock.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/code/__snapshots__/CodeElement.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/code/__snapshots__/PreviewCodeBlock.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/code/__snapshots__/PreviewCodeElement.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/colors/__snapshots__/ColorElement.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/colors/__snapshots__/PreviewColorElement.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/elements/Timestamp/__snapshots__/DateTimeFormats.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/elements/Timestamp/__snapshots__/RelativeTime.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/elements/Timestamp/__snapshots__/Time.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/elements/Timestamp/__snapshots__/Timestamp.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/elements/Timestamp/__snapshots__/TimestampWrapper.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/elements/__snapshots__/BoldSpan.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/elements/__snapshots__/ImageElement.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/elements/__snapshots__/InlineElements.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/elements/__snapshots__/ItalicSpan.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/elements/__snapshots__/LinkSpan.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/elements/__snapshots__/PlainSpan.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/elements/__snapshots__/PreviewInlineElements.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/elements/__snapshots__/SpoilerSpan.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/elements/__snapshots__/StrikeSpan.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/emoji/__snapshots__/BigEmojiBlock.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/emoji/__snapshots__/BigEmojiElement.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/emoji/__snapshots__/Emoji.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/emoji/__snapshots__/EmojiRenderer.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/emoji/__snapshots__/PreviewBigEmojiBlock.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/emoji/__snapshots__/PreviewEmojiElement.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/katex/__snapshots__/KatexBlock.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/katex/__snapshots__/KatexElement.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/katex/__snapshots__/KatexErrorBoundary.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/katex/__snapshots__/PreviewKatexBlock.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/katex/__snapshots__/PreviewKatexElement.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/mentions/__snapshots__/ChannelMentionElement.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/mentions/__snapshots__/PreviewChannelMentionElement.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/mentions/__snapshots__/PreviewUserMentionElement.spec.tsx.snapis excluded by!**/*.snappackages/gazzodown/src/mentions/__snapshots__/UserMentionElement.spec.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (57)
packages/gazzodown/src/Markup.spec.tsxpackages/gazzodown/src/Markup.stories.tsxpackages/gazzodown/src/PreviewMarkup.spec.tsxpackages/gazzodown/src/blocks/HeadingBlock.spec.tsxpackages/gazzodown/src/blocks/OrderedListBlock.spec.tsxpackages/gazzodown/src/blocks/ParagraphBlock.spec.tsxpackages/gazzodown/src/blocks/QuoteBlock.spec.tsxpackages/gazzodown/src/blocks/SpoilerBlock.spec.tsxpackages/gazzodown/src/blocks/TaskListBlock.spec.tsxpackages/gazzodown/src/blocks/UnorderedListBlock.spec.tsxpackages/gazzodown/src/code/CodeBlock.spec.tsxpackages/gazzodown/src/code/CodeElement.spec.tsxpackages/gazzodown/src/code/PreviewCodeBlock.spec.tsxpackages/gazzodown/src/code/PreviewCodeElement.spec.tsxpackages/gazzodown/src/colors/ColorElement.spec.tsxpackages/gazzodown/src/colors/PreviewColorElement.spec.tsxpackages/gazzodown/src/elements/BoldSpan.spec.tsxpackages/gazzodown/src/elements/ImageElement.spec.tsxpackages/gazzodown/src/elements/InlineElements.spec.tsxpackages/gazzodown/src/elements/ItalicSpan.spec.tsxpackages/gazzodown/src/elements/LinkSpan.spec.tsxpackages/gazzodown/src/elements/PlainSpan.spec.tsxpackages/gazzodown/src/elements/PreviewInlineElements.spec.tsxpackages/gazzodown/src/elements/SpoilerSpan.spec.tsxpackages/gazzodown/src/elements/StrikeSpan.spec.tsxpackages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsxpackages/gazzodown/src/elements/Timestamp/FullDate.tsxpackages/gazzodown/src/elements/Timestamp/FullDateLong.tsxpackages/gazzodown/src/elements/Timestamp/LongDate.tsxpackages/gazzodown/src/elements/Timestamp/LongTime.tsxpackages/gazzodown/src/elements/Timestamp/RelativeTime.spec.tsxpackages/gazzodown/src/elements/Timestamp/RelativeTime.tsxpackages/gazzodown/src/elements/Timestamp/ShortDate.tsxpackages/gazzodown/src/elements/Timestamp/ShortTime.tsxpackages/gazzodown/src/elements/Timestamp/Time.spec.tsxpackages/gazzodown/src/elements/Timestamp/Time.tsxpackages/gazzodown/src/elements/Timestamp/Timestamp.spec.tsxpackages/gazzodown/src/elements/Timestamp/Timestamp.tsxpackages/gazzodown/src/elements/Timestamp/TimestampWrapper.spec.tsxpackages/gazzodown/src/elements/Timestamp/TimestampWrapper.tsxpackages/gazzodown/src/elements/Timestamp/index.tspackages/gazzodown/src/elements/Timestamp/index.tsxpackages/gazzodown/src/emoji/BigEmojiBlock.spec.tsxpackages/gazzodown/src/emoji/BigEmojiElement.spec.tsxpackages/gazzodown/src/emoji/Emoji.spec.tsxpackages/gazzodown/src/emoji/EmojiRenderer.spec.tsxpackages/gazzodown/src/emoji/PreviewBigEmojiBlock.spec.tsxpackages/gazzodown/src/emoji/PreviewEmojiElement.spec.tsxpackages/gazzodown/src/katex/KatexBlock.spec.tsxpackages/gazzodown/src/katex/KatexElement.spec.tsxpackages/gazzodown/src/katex/KatexErrorBoundary.spec.tsxpackages/gazzodown/src/katex/PreviewKatexBlock.spec.tsxpackages/gazzodown/src/katex/PreviewKatexElement.spec.tsxpackages/gazzodown/src/mentions/ChannelMentionElement.spec.tsxpackages/gazzodown/src/mentions/PreviewChannelMentionElement.spec.tsxpackages/gazzodown/src/mentions/PreviewUserMentionElement.spec.tsxpackages/gazzodown/src/mentions/UserMentionElement.spec.tsx
💤 Files with no reviewable changes (1)
- packages/gazzodown/src/elements/Timestamp/index.tsx
🚧 Files skipped from review as they are similar to previous changes (42)
- packages/gazzodown/src/elements/Timestamp/TimestampWrapper.spec.tsx
- packages/gazzodown/src/emoji/BigEmojiBlock.spec.tsx
- packages/gazzodown/src/blocks/TaskListBlock.spec.tsx
- packages/gazzodown/src/blocks/OrderedListBlock.spec.tsx
- packages/gazzodown/src/blocks/UnorderedListBlock.spec.tsx
- packages/gazzodown/src/elements/PlainSpan.spec.tsx
- packages/gazzodown/src/code/CodeElement.spec.tsx
- packages/gazzodown/src/elements/Timestamp/Time.spec.tsx
- packages/gazzodown/src/PreviewMarkup.spec.tsx
- packages/gazzodown/src/elements/InlineElements.spec.tsx
- packages/gazzodown/src/elements/Timestamp/index.ts
- packages/gazzodown/src/colors/ColorElement.spec.tsx
- packages/gazzodown/src/blocks/ParagraphBlock.spec.tsx
- packages/gazzodown/src/elements/SpoilerSpan.spec.tsx
- packages/gazzodown/src/katex/KatexErrorBoundary.spec.tsx
- packages/gazzodown/src/elements/PreviewInlineElements.spec.tsx
- packages/gazzodown/src/katex/KatexElement.spec.tsx
- packages/gazzodown/src/emoji/EmojiRenderer.spec.tsx
- packages/gazzodown/src/emoji/PreviewBigEmojiBlock.spec.tsx
- packages/gazzodown/src/blocks/QuoteBlock.spec.tsx
- packages/gazzodown/src/emoji/PreviewEmojiElement.spec.tsx
- packages/gazzodown/src/elements/ItalicSpan.spec.tsx
- packages/gazzodown/src/katex/KatexBlock.spec.tsx
- packages/gazzodown/src/emoji/BigEmojiElement.spec.tsx
- packages/gazzodown/src/elements/LinkSpan.spec.tsx
- packages/gazzodown/src/elements/Timestamp/FullDate.tsx
- packages/gazzodown/src/elements/Timestamp/RelativeTime.spec.tsx
- packages/gazzodown/src/elements/ImageElement.spec.tsx
- packages/gazzodown/src/elements/Timestamp/ShortTime.tsx
- packages/gazzodown/src/elements/Timestamp/TimestampWrapper.tsx
- packages/gazzodown/src/blocks/HeadingBlock.spec.tsx
- packages/gazzodown/src/elements/Timestamp/LongTime.tsx
- packages/gazzodown/src/code/CodeBlock.spec.tsx
- packages/gazzodown/src/mentions/PreviewUserMentionElement.spec.tsx
- packages/gazzodown/src/katex/PreviewKatexElement.spec.tsx
- packages/gazzodown/src/emoji/Emoji.spec.tsx
- packages/gazzodown/src/elements/Timestamp/Timestamp.spec.tsx
- packages/gazzodown/src/mentions/PreviewChannelMentionElement.spec.tsx
- packages/gazzodown/src/code/PreviewCodeBlock.spec.tsx
- packages/gazzodown/src/elements/BoldSpan.spec.tsx
- packages/gazzodown/src/elements/Timestamp/Time.tsx
- packages/gazzodown/src/elements/Timestamp/FullDateLong.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
packages/gazzodown/src/Markup.spec.tsxpackages/gazzodown/src/elements/Timestamp/Timestamp.tsxpackages/gazzodown/src/elements/Timestamp/LongDate.tsxpackages/gazzodown/src/elements/Timestamp/ShortDate.tsxpackages/gazzodown/src/mentions/ChannelMentionElement.spec.tsxpackages/gazzodown/src/katex/PreviewKatexBlock.spec.tsxpackages/gazzodown/src/code/PreviewCodeElement.spec.tsxpackages/gazzodown/src/Markup.stories.tsxpackages/gazzodown/src/blocks/SpoilerBlock.spec.tsxpackages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsxpackages/gazzodown/src/mentions/UserMentionElement.spec.tsxpackages/gazzodown/src/elements/Timestamp/RelativeTime.tsxpackages/gazzodown/src/elements/StrikeSpan.spec.tsxpackages/gazzodown/src/colors/PreviewColorElement.spec.tsx
🧠 Learnings (27)
📓 Common learnings
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsx:20-23
Timestamp: 2026-03-06T18:09:17.867Z
Learning: In the RocketChat/Rocket.Chat gazzodown package (`packages/gazzodown`), tests are intended to run under the UTC timezone, but as of PR `#39397` this is NOT yet explicitly enforced in `jest.config.ts` or the `package.json` test scripts (which just run `jest` without `TZ=UTC`). To make timezone-sensitive snapshot tests reliable across all environments, `TZ=UTC` should be added to the test scripts in `package.json` or to `jest.config.ts` via `testEnvironmentOptions.timezone`. Without explicit UTC enforcement, snapshot tests involving date-fns formatted output or `toLocaleString()` will fail for contributors in non-UTC timezones.
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/elements/Timestamp/RelativeTime.spec.tsx:63-70
Timestamp: 2026-03-06T18:02:20.381Z
Learning: In RocketChat/Rocket.Chat, tests in the `packages/gazzodown` package (and likely the broader test suite) are always expected to run in the UTC timezone. This makes `toLocaleString()` output deterministic, so snapshot tests that include locale/timezone-sensitive content (such as `title` attributes from `toLocaleString()`) are stable and do not need to be replaced with targeted assertions.
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `expect` matchers for assertions (`toEqual`, `toContain`, `toBeTruthy`, `toHaveLength`, etc.) instead of `assert` statements in Playwright tests
Applied to files:
packages/gazzodown/src/Markup.spec.tsxpackages/gazzodown/src/mentions/ChannelMentionElement.spec.tsxpackages/gazzodown/src/katex/PreviewKatexBlock.spec.tsxpackages/gazzodown/src/code/PreviewCodeElement.spec.tsxpackages/gazzodown/src/blocks/SpoilerBlock.spec.tsxpackages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsxpackages/gazzodown/src/mentions/UserMentionElement.spec.tsxpackages/gazzodown/src/elements/StrikeSpan.spec.tsxpackages/gazzodown/src/colors/PreviewColorElement.spec.tsx
📚 Learning: 2026-03-06T18:02:11.732Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/elements/Timestamp/RelativeTime.spec.tsx:63-70
Timestamp: 2026-03-06T18:02:11.732Z
Learning: In Rocket.Chat, tests within the gazzodown package (and the broader test suite) should run in UTC to make locale/timezone-sensitive outputs from toLocaleString() deterministic. Ensure snapshot tests that capture values like title attributes based on toLocaleString() remain stable by running tests in UTC. Actions you can take: - configure the test environment to TZ=UTC (e.g., in CI or local npm/yarn test scripts), - optionally set process.env.TZ = 'UTC' in Jest/setupFiles or the test bootstrap, - avoid asserting locale-sensitive strings directly; prefer targeted assertions or mocks where necessary. Apply this guideline to all spec files under packages/gazzodown/src that involve timestamp rendering (pattern above).
Applied to files:
packages/gazzodown/src/Markup.spec.tsxpackages/gazzodown/src/mentions/ChannelMentionElement.spec.tsxpackages/gazzodown/src/katex/PreviewKatexBlock.spec.tsxpackages/gazzodown/src/code/PreviewCodeElement.spec.tsxpackages/gazzodown/src/blocks/SpoilerBlock.spec.tsxpackages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsxpackages/gazzodown/src/mentions/UserMentionElement.spec.tsxpackages/gazzodown/src/elements/StrikeSpan.spec.tsxpackages/gazzodown/src/colors/PreviewColorElement.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (`test`, `page`, `expect`) for consistency in test files
Applied to files:
packages/gazzodown/src/Markup.spec.tsxpackages/gazzodown/src/mentions/ChannelMentionElement.spec.tsxpackages/gazzodown/src/katex/PreviewKatexBlock.spec.tsxpackages/gazzodown/src/code/PreviewCodeElement.spec.tsxpackages/gazzodown/src/blocks/SpoilerBlock.spec.tsxpackages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsxpackages/gazzodown/src/mentions/UserMentionElement.spec.tsxpackages/gazzodown/src/elements/StrikeSpan.spec.tsxpackages/gazzodown/src/colors/PreviewColorElement.spec.tsx
📚 Learning: 2026-03-06T18:09:17.867Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsx:20-23
Timestamp: 2026-03-06T18:09:17.867Z
Learning: In the RocketChat/Rocket.Chat gazzodown package (`packages/gazzodown`), tests are intended to run under the UTC timezone, but as of PR `#39397` this is NOT yet explicitly enforced in `jest.config.ts` or the `package.json` test scripts (which just run `jest` without `TZ=UTC`). To make timezone-sensitive snapshot tests reliable across all environments, `TZ=UTC` should be added to the test scripts in `package.json` or to `jest.config.ts` via `testEnvironmentOptions.timezone`. Without explicit UTC enforcement, snapshot tests involving date-fns formatted output or `toLocaleString()` will fail for contributors in non-UTC timezones.
Applied to files:
packages/gazzodown/src/Markup.spec.tsxpackages/gazzodown/src/elements/Timestamp/Timestamp.tsxpackages/gazzodown/src/elements/Timestamp/LongDate.tsxpackages/gazzodown/src/elements/Timestamp/ShortDate.tsxpackages/gazzodown/src/katex/PreviewKatexBlock.spec.tsxpackages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsxpackages/gazzodown/src/mentions/UserMentionElement.spec.tsxpackages/gazzodown/src/elements/Timestamp/RelativeTime.tsxpackages/gazzodown/src/elements/StrikeSpan.spec.tsxpackages/gazzodown/src/colors/PreviewColorElement.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Prefer web-first assertions (`toBeVisible`, `toHaveText`, etc.) in Playwright tests
Applied to files:
packages/gazzodown/src/Markup.spec.tsxpackages/gazzodown/src/katex/PreviewKatexBlock.spec.tsxpackages/gazzodown/src/code/PreviewCodeElement.spec.tsxpackages/gazzodown/src/blocks/SpoilerBlock.spec.tsxpackages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsxpackages/gazzodown/src/mentions/UserMentionElement.spec.tsxpackages/gazzodown/src/elements/StrikeSpan.spec.tsxpackages/gazzodown/src/colors/PreviewColorElement.spec.tsx
📚 Learning: 2025-12-10T21:00:54.909Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37091
File: ee/packages/abac/jest.config.ts:4-7
Timestamp: 2025-12-10T21:00:54.909Z
Learning: Rocket.Chat monorepo: Jest testMatch pattern '<rootDir>/src/**/*.spec.(ts|js|mjs)' is valid in this repo and used across multiple packages (e.g., packages/tools, ee/packages/omnichannel-services). Do not flag it as invalid in future reviews.
Applied to files:
packages/gazzodown/src/Markup.spec.tsxpackages/gazzodown/src/mentions/ChannelMentionElement.spec.tsxpackages/gazzodown/src/katex/PreviewKatexBlock.spec.tsxpackages/gazzodown/src/code/PreviewCodeElement.spec.tsxpackages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsxpackages/gazzodown/src/mentions/UserMentionElement.spec.tsxpackages/gazzodown/src/elements/StrikeSpan.spec.tsxpackages/gazzodown/src/colors/PreviewColorElement.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to **/*.spec.ts : Use descriptive test names that clearly communicate expected behavior in Playwright tests
Applied to files:
packages/gazzodown/src/Markup.spec.tsxpackages/gazzodown/src/katex/PreviewKatexBlock.spec.tsxpackages/gazzodown/src/code/PreviewCodeElement.spec.tsxpackages/gazzodown/src/blocks/SpoilerBlock.spec.tsxpackages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsxpackages/gazzodown/src/mentions/UserMentionElement.spec.tsxpackages/gazzodown/src/elements/StrikeSpan.spec.tsxpackages/gazzodown/src/colors/PreviewColorElement.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Maintain test isolation between test cases in Playwright tests
Applied to files:
packages/gazzodown/src/Markup.spec.tsxpackages/gazzodown/src/katex/PreviewKatexBlock.spec.tsxpackages/gazzodown/src/code/PreviewCodeElement.spec.tsxpackages/gazzodown/src/blocks/SpoilerBlock.spec.tsxpackages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsxpackages/gazzodown/src/elements/StrikeSpan.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
packages/gazzodown/src/Markup.spec.tsxpackages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Implement proper wait strategies for dynamic content in Playwright tests
Applied to files:
packages/gazzodown/src/Markup.spec.tsxpackages/gazzodown/src/code/PreviewCodeElement.spec.tsxpackages/gazzodown/src/blocks/SpoilerBlock.spec.tsxpackages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsxpackages/gazzodown/src/elements/StrikeSpan.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure clean state for each test execution in Playwright tests
Applied to files:
packages/gazzodown/src/Markup.spec.tsxpackages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsx
📚 Learning: 2025-12-16T17:29:45.163Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37834
File: apps/meteor/tests/e2e/page-objects/fragments/admin-flextab-emoji.ts:12-22
Timestamp: 2025-12-16T17:29:45.163Z
Learning: In page object files under `apps/meteor/tests/e2e/page-objects/`, always import `expect` from `../../utils/test` (Playwright's async expect), not from Jest. Jest's `expect` has a synchronous signature and will cause TypeScript errors when used with web-first assertions like `toBeVisible()`.
Applied to files:
packages/gazzodown/src/Markup.spec.tsxpackages/gazzodown/src/code/PreviewCodeElement.spec.tsxpackages/gazzodown/src/blocks/SpoilerBlock.spec.tsx
📚 Learning: 2026-02-24T19:36:55.089Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/page-objects/fragments/home-content.ts:60-82
Timestamp: 2026-02-24T19:36:55.089Z
Learning: In RocketChat/Rocket.Chat e2e tests (apps/meteor/tests/e2e/page-objects/fragments/home-content.ts), thread message preview listitems do not have aria-roledescription="message", so lastThreadMessagePreview locator cannot be scoped to messageListItems (which filters for aria-roledescription="message"). It should remain scoped to page.getByRole('listitem') or mainMessageList.getByRole('listitem').
Applied to files:
packages/gazzodown/src/Markup.spec.tsxpackages/gazzodown/src/code/PreviewCodeElement.spec.tsxpackages/gazzodown/src/blocks/SpoilerBlock.spec.tsxpackages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsxpackages/gazzodown/src/colors/PreviewColorElement.spec.tsx
📚 Learning: 2026-03-06T18:10:15.268Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/code/CodeBlock.spec.tsx:47-68
Timestamp: 2026-03-06T18:10:15.268Z
Learning: In tests (especially those using testing-library/dom/jsdom) for Rocket.Chat components, the HTML <code> element has an implicit ARIA role of 'code'. Therefore, screen.getByRole('code') or screen.findByRole('code') will locate <code> elements even without a role attribute. Do not flag findByRole('code') as invalid in reviews; prefer using the implicit role instead of adding role="code" unless necessary for accessibility.
Applied to files:
packages/gazzodown/src/Markup.spec.tsxpackages/gazzodown/src/mentions/ChannelMentionElement.spec.tsxpackages/gazzodown/src/katex/PreviewKatexBlock.spec.tsxpackages/gazzodown/src/code/PreviewCodeElement.spec.tsxpackages/gazzodown/src/blocks/SpoilerBlock.spec.tsxpackages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsxpackages/gazzodown/src/mentions/UserMentionElement.spec.tsxpackages/gazzodown/src/elements/StrikeSpan.spec.tsxpackages/gazzodown/src/colors/PreviewColorElement.spec.tsx
📚 Learning: 2026-03-06T18:02:20.381Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/elements/Timestamp/RelativeTime.spec.tsx:63-70
Timestamp: 2026-03-06T18:02:20.381Z
Learning: In RocketChat/Rocket.Chat, tests in the `packages/gazzodown` package (and likely the broader test suite) are always expected to run in the UTC timezone. This makes `toLocaleString()` output deterministic, so snapshot tests that include locale/timezone-sensitive content (such as `title` attributes from `toLocaleString()`) are stable and do not need to be replaced with targeted assertions.
Applied to files:
packages/gazzodown/src/elements/Timestamp/LongDate.tsxpackages/gazzodown/src/elements/Timestamp/RelativeTime.tsx
📚 Learning: 2025-11-24T17:08:26.531Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/test-cases.mdc:0-0
Timestamp: 2025-11-24T17:08:26.531Z
Learning: Applies to **/*test-case*.md : Write test cases in Markdown format following the standardized template with ALL necessary components: Title, Description, Preconditions, Type, Steps, and Expected Result
Applied to files:
packages/gazzodown/src/katex/PreviewKatexBlock.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to **/*.{ts,tsx,js} : Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Applied to files:
packages/gazzodown/src/code/PreviewCodeElement.spec.tsxpackages/gazzodown/src/elements/StrikeSpan.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `test.step()` for complex test scenarios to improve organization in Playwright tests
Applied to files:
packages/gazzodown/src/code/PreviewCodeElement.spec.tsxpackages/gazzodown/src/blocks/SpoilerBlock.spec.tsxpackages/gazzodown/src/elements/StrikeSpan.spec.tsx
📚 Learning: 2025-09-23T19:22:59.217Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 36987
File: apps/meteor/tests/e2e/page-objects/fragments/room-toolbar.ts:10-20
Timestamp: 2025-09-23T19:22:59.217Z
Learning: In Playwright e2e tests, prefer stable selectors like data-qa-id attributes over localized text in getByRole() or getByText() calls to prevent test failures when UI language changes. Test translations separately by validating actual text content after ensuring UI interactions work with stable selectors.
Applied to files:
packages/gazzodown/src/code/PreviewCodeElement.spec.tsxpackages/gazzodown/src/blocks/SpoilerBlock.spec.tsxpackages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Group related tests in the same file
Applied to files:
packages/gazzodown/src/blocks/SpoilerBlock.spec.tsxpackages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : All test files must be created in `apps/meteor/tests/e2e/` directory
Applied to files:
packages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `page.waitFor()` with specific conditions instead of hardcoded timeouts in Playwright tests
Applied to files:
packages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,spec.ts} : Follow Page Object Model pattern consistently in Playwright tests
Applied to files:
packages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsx
📚 Learning: 2025-11-19T12:32:29.696Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37547
File: packages/i18n/src/locales/en.i18n.json:634-634
Timestamp: 2025-11-19T12:32:29.696Z
Learning: Repo: RocketChat/Rocket.Chat
Context: i18n workflow
Learning: In this repository, new translation keys should be added to packages/i18n/src/locales/en.i18n.json only; other locale files are populated via the external translation pipeline and/or fall back to English. Do not request adding the same key to all locale files in future reviews.
Applied to files:
packages/gazzodown/src/elements/Timestamp/RelativeTime.tsx
📚 Learning: 2025-11-19T18:20:07.720Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37419
File: packages/i18n/src/locales/en.i18n.json:918-921
Timestamp: 2025-11-19T18:20:07.720Z
Learning: Repo: RocketChat/Rocket.Chat — i18n/formatting
Learning: This repository uses a custom message formatting parser in UI blocks/messages; do not assume standard Markdown rules. For keys like Call_ended_bold, Call_not_answered_bold, Call_failed_bold, and Call_transferred_bold in packages/i18n/src/locales/en.i18n.json, retain the existing single-asterisk emphasis unless maintainers request otherwise.
Applied to files:
packages/gazzodown/src/elements/Timestamp/RelativeTime.tsx
📚 Learning: 2026-01-17T01:51:47.764Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38219
File: packages/core-typings/src/cloud/Announcement.ts:5-6
Timestamp: 2026-01-17T01:51:47.764Z
Learning: In packages/core-typings/src/cloud/Announcement.ts, the AnnouncementSchema.createdBy field intentionally overrides IBannerSchema.createdBy (object with _id and optional username) with a string enum ['cloud', 'system'] to match existing runtime behavior. This is documented as technical debt with a FIXME comment at apps/meteor/app/cloud/server/functions/syncWorkspace/handleCommsSync.ts:53 and should not be flagged as an error until the runtime behavior is corrected.
Applied to files:
packages/gazzodown/src/elements/Timestamp/RelativeTime.tsx
🔇 Additional comments (17)
packages/gazzodown/src/Markup.stories.tsx (1)
252-256: LGTM!The explicit type annotation
{ msg }: { msg: string }is technically redundant since TypeScript infers the type fromStoryFn<{ msg: string }>, but it doesn't cause any issues and aligns with the PR's goal of tightening types throughout the package.packages/gazzodown/src/colors/PreviewColorElement.spec.tsx (2)
1-41: Test suite looks solid overall.The tests provide good coverage for the
PreviewColorElementcomponent, correctly verifying:
- Alpha omission when
a === 255- Alpha inclusion when
a !== 255- Full white and zero alpha edge cases
- Snapshot stability for both rendering paths
17-20: Padding test uses only zeros — doesn't validate non-zero single-digit padding.The test name claims to verify leading-zero padding, but
r=0, g=0, b=0all produce"00"regardless of whetherpadStartworks correctly for values like1→"01"or15→"0f".packages/gazzodown/src/elements/StrikeSpan.spec.tsx (1)
1-24: LGTM!The test suite is well-structured with clear, descriptive test names. The use of
closest()for verifying DOM hierarchy is a reasonable approach for elements like<del>that lack a dedicated ARIA role, and the eslint-disable comments are appropriately scoped to just those lines.packages/gazzodown/src/Markup.spec.tsx (5)
11-105: LGTM!The snapshot additions for empty render, emoji blocks, paragraph, and heading tests follow best practices - behavioral assertions come first, followed by snapshot assertions. The container destructuring pattern is used correctly.
108-193: LGTM!List tests (unordered, ordered, task) properly verify structural elements and content before taking snapshots. The checkbox state assertions in the task list test provide good behavioral coverage alongside the snapshot.
196-267: LGTM!Good async handling with
findByRolefor Suspense-wrapped code blocks. ThegetByRole('code')usage is valid since<code>elements have an implicit ARIA role of 'code'. Blockquote test properly verifies content presence before snapshotting.
285-291: DOM mutation before snapshotting affects test reliability.The
container.querySelector('math')?.remove()call mutates the live rendered DOM before taking the snapshot. This changes the subject under test and can mask regressions. Consider cloning the container before removing the<math>element for snapshot purposes.
294-363: LGTM!Line break and emoji preference tests are well-structured. The emoji preference tests correctly verify that the rendering respects context settings before capturing snapshots.
packages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsx (2)
1-24: LGTM! Well-structured test suite with consistent patterns.The tests correctly verify that each component sets the
dateTimeattribute to the ISO string representation and include snapshot coverage. The direct DOM access viadocument.querySelector('time')is appropriate here since<time>elements have no implicit ARIA role.
20-23: Snapshot tests remain fragile without explicit UTC enforcement.The previous review discussion noted that while tests are intended to run under UTC,
TZ=UTCis not explicitly enforced injest.config.tsor the test scripts. Contributors in non-UTC timezones will see snapshot failures. Based on learnings, consider addingTZ=UTCto the test scripts inpackage.jsonor configuringtestEnvironmentOptions.timezoneinjest.config.ts.Also applies to: 36-39, 52-55, 68-71, 84-87, 100-103
packages/gazzodown/src/code/PreviewCodeElement.spec.tsx (2)
6-9: Good direct assertion for the rendered code.This checks the visible output explicitly, which makes the test more valuable than relying on snapshots alone.
16-18: Nice lightweight snapshot coverage here.Using a small representative sample keeps this snapshot focused and complementary to the behavioral checks above.
packages/gazzodown/src/katex/PreviewKatexBlock.spec.tsx (1)
6-13: Nice focused coverage for the preview contract.These assertions line up with
PreviewKatexBlockreturning rawcodetext instead of KaTeX markup.packages/gazzodown/src/mentions/UserMentionElement.spec.tsx (1)
7-82: Branch coverage is solid here.These assertions exercise the key
UserMentionElementpaths: unresolved mentions, special mentions, resolved mentions, real-name rendering, and symbol toggling.packages/gazzodown/src/mentions/ChannelMentionElement.spec.tsx (1)
7-58: Behavioral coverage looks good.This suite hits the important
ChannelMentionElementbranches: unresolved rendering, resolved rendering, symbol prefixing, and thefnamefallback path.packages/gazzodown/src/elements/Timestamp/ShortDate.tsx (1)
9-9: LGTM.
'P'keeps this variant date-only and matches thedtimestamp branch cleanly.
tassoevan
force-pushed
the
test/gazzodown-snapshots
branch
2 times, most recently
from
e2d29e0 to
6853f7c
Compare
tassoevan
force-pushed
the
test/gazzodown-snapshots
branch
from
6853f7c to
472268d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes (including videos or screenshots)
It expands test coverage for
@rocket.chat/gazzodown, including snapshots.Further comments
It became fragile to side changes.
Summary by CodeRabbit
New Features
Enhancements
Tests