test: act on review — observe the wiring, and stop the soft assertions

All five points were right.

* The gallery's Toasts copy still read "Bottom right" after the container
  moved.  Anyone using that page as a visual checklist would have looked in
  the wrong corner and concluded the change had not landed.

* Nothing observed ThemeProvider's own wiring.  Both suites mount their own
  `<Notifications position='top-right' limit={5}/>`, so moving the container
  back, or dropping it, would have failed neither -- while the comments
  claimed "container missing while call sites still fire" as the failure
  already shipped once.  Three tests in ThemeProvider.test.js now raise a
  real toast through the provider and assert it appears, and where.

  Asserted through a toast rather than by querying for a container: Mantine
  renders an empty container for every position it supports, so the obvious
  selector returns `top-center` no matter how the provider is configured.
  That is what the first version of this test did, and it failed for that
  reason rather than the intended one.

  The five-at-once test is honest about what it covers: Mantine's own
  default limit is also 5, so it does not prove the prop is there.  It
  catches the limit being raised.

* The default duration was untested.  Sleeping 150ms and finding the toast
  still up rules out "vanishes next tick" and nothing else -- it passes for
  a 200ms default, for `false`, and for Mantine's own 4000ms.  Read the
  deadline back off Mantine's store instead: 5000 when unset, the caller's
  number when given, `false` for 0.

* Space activates a followable toast in `toast.ts` and only Enter was
  covered, so dropping that branch stayed green.

* Colour assertions used `toContain('red')`, which any declaration
  containing those three letters satisfies.  They match the whole custom
  property now.

Planted four regressions to check the new tests: position, limit, the
5000ms default, and the Space branch.  Three went red; the limit one did
not, for the reason recorded above.
This commit is contained in:
Roland 2026-08-01 15:22:39 -06:00
parent 664c7d2219
commit b36f7beeb8
No known key found for this signature in database
GPG key ID: 1BA620CE6C9A4680
3 changed files with 119 additions and 12 deletions

View file

@ -1,7 +1,7 @@
import fs from 'fs';
import path from 'path';
import React, {useContext} from 'react';
import {act, render, screen} from '@testing-library/react';
import {act, render, screen, waitFor} from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import {ThemeContext} from '../contexts/contexts';
import {
@ -16,6 +16,7 @@ import {
themeSessionKey,
} from './Theme';
import {mediaFilterSessionKey, resolveMediaFilter, themeMediaFilter} from '../themes/names';
import {clearToasts, toast} from './ui';
// Lets a test drive `prefers-color-scheme` and assert on what the provider applied.
let prefersDark = false;
@ -302,3 +303,69 @@ describe('theme helpers', () => {
expect(isDarkTheme(amberTheme)).toBe(true);
});
});
describe('the toast container ThemeProvider mounts', () => {
/*
* Observing the wiring rather than re-stating it.
*
* `toast.test.js` and `ui-layout.cy.js` each mount their own `<Notifications>` with the
* position and limit they expect, so neither would notice if this file moved the container
* back, changed its limit, or dropped it altogether -- and a missing container while
* thirteen call sites keep firing is the failure that has already shipped once. Those
* suites test `toast()`; this tests that the app has somewhere to put what it produces.
*/
beforeEach(() => {
prefersDark = false;
localStorage.clear();
mockMatchMedia();
});
afterEach(() => act(() => clearToasts()));
const notifications = () =>
document.querySelectorAll('[class*="mantine-Notification-root"]');
it('shows a toast raised by a call site', async () => {
// The regression that already shipped: no container, thirteen call sites still firing,
// nothing thrown and nothing shown.
renderProvider();
act(() => toast({title: 'Refresh started'}));
expect(await screen.findByText('Refresh started')).toBeInTheDocument();
});
it('puts it in the top right', async () => {
/*
* Asserted through a real toast rather than by finding a container. Mantine renders an
* empty container for every position it supports, so querying for one returns whichever
* comes first in the DOM -- `top-center` -- whatever the provider was configured with.
*/
renderProvider();
act(() => toast({title: 'Somewhere'}));
const notification = await screen.findByText('Somewhere');
expect(notification.closest('[class*="mantine-Notifications-root"]'))
.toHaveAttribute('data-position', 'top-right');
});
it('shows five at once and queues the rest', async () => {
/*
* The user-visible behaviour, not the prop: Mantine's own default limit is also 5, so
* this stays green if `limit={5}` is deleted from the provider. It is here to catch a
* limit being *raised* -- a background task that reports twenty times should not bury
* the page -- rather than to prove the prop is present.
*/
renderProvider();
act(() => {
for (let i = 1; i <= 7; i += 1) {
toast({title: `Toast ${i}`});
}
});
await waitFor(() => expect(notifications()).toHaveLength(5));
});
});

View file

@ -288,7 +288,7 @@ export function ThemeSamplePage() {
</Row>
<p style={{fontSize: 12, color: 'var(--muted)', marginBottom: 0}}>
Bottom right, five at a time, five seconds each unless the caller says
Top right, five at a time, five seconds each unless the caller says
otherwise. Check each type in all four themes: night has no second hue to
spend, so info, success, warning and error cannot be told apart by colour
there.

View file

@ -4,6 +4,7 @@ import userEvent from '@testing-library/user-event';
import {MantineProvider} from '@mantine/core';
import {Notifications} from '@mantine/notifications';
import {cssVariablesResolver, mantineTheme} from '../../themes/mantine';
import {notificationsStore} from '@mantine/notifications';
import {clearToasts, toast} from './toast';
/*
@ -111,9 +112,10 @@ describe('toast', () => {
showToast({type: 'error', title: 'Download failed'});
await waitFor(() => expect(visibleToasts()).toHaveLength(1));
// Mantine writes the resolved colour onto the element as a custom property.
const notification = visibleToasts()[0];
expect(notification.getAttribute('style')).toContain('red');
// The whole custom property, not a substring: `toContain('red')` would also be
// satisfied by any other declaration that happens to carry those three letters.
expect(visibleToasts()[0].getAttribute('style'))
.toMatch(/--notification-color:\s*var\(--mantine-color-red-/);
});
it('gives each type a colour of its own', async () => {
@ -142,7 +144,8 @@ describe('toast', () => {
showToast({title: 'Refresh started'});
await waitFor(() => expect(visibleToasts()).toHaveLength(1));
expect(visibleToasts()[0].getAttribute('style')).toContain('blue');
expect(visibleToasts()[0].getAttribute('style'))
.toMatch(/--notification-color:\s*var\(--mantine-color-blue-/);
});
it('survives being called with nothing at all', () => {
@ -203,6 +206,20 @@ describe('a toast that can be clicked', () => {
expect(onClick).toHaveBeenCalledTimes(1);
});
it('can also be activated with Space', async () => {
// `toast.ts` handles Enter and Space; only Enter was covered, so dropping the Space
// branch -- or switching to `event.code` -- would have stayed green.
const onClick = jest.fn();
withToasts();
showToast({title: 'Screenshot Generated', onClick});
await waitFor(() => expect(visibleToasts()).toHaveLength(1));
visibleToasts()[0].focus();
await userEvent.keyboard(' ');
expect(onClick).toHaveBeenCalledTimes(1);
});
it('looks clickable only when it is', async () => {
withToasts();
@ -244,15 +261,38 @@ describe('how long a toast stays up', () => {
await waitFor(() => expect(screen.queryByText('Saved')).not.toBeInTheDocument());
});
it('is still up a moment after being shown, when no time is given', async () => {
// The default is five seconds, which is not worth waiting for; what is worth asserting
// is that a caller who names no time does not get a toast that vanishes instantly.
/*
* The dismissal deadline `toast()` hands Mantine, read back off its store.
*
* Waiting is no good for this. Sleeping 150ms and finding the toast still there catches
* "vanishes on the next tick" and nothing else -- it passes just as happily for a 200ms
* default, a 1000ms one, `false`, or Mantine's own container default of 4000ms. Waiting
* the full five seconds instead would put five seconds into every run. The store holds
* exactly what was passed, which is the thing worth asserting.
*/
const autoCloseOf = (options) => {
showToast(options);
const [notification] = notificationsStore.getState().notifications;
return notification.autoClose;
};
it('gives a toast five seconds when the caller names no time', () => {
withToasts();
showToast({title: 'Refresh started'});
await new Promise(resolve => setTimeout(resolve, 150));
expect(autoCloseOf({title: 'Refresh started'})).toBe(5000);
});
expect(screen.getByText('Refresh started')).toBeInTheDocument();
it('passes a time the caller chose straight through', () => {
withToasts();
expect(autoCloseOf({title: 'Brief', time: 250})).toBe(250);
});
it('turns time 0 into no deadline at all', () => {
// Not 0. Mantine reads a number as a delay, so a literal 0 dismisses on the next tick.
withToasts();
expect(autoCloseOf({title: 'Download failed', time: 0})).toBe(false);
});
it('stays until dismissed when the caller asks for time 0', async () => {