mirror of
https://github.com/lrnselfreliance/wrolpi.git
synced 2026-10-26 19:01:39 -04:00
test: act on review — the browser test named LoadStatistic but never mounted it
The suite was titled "a load reading ranks itself", commented as covering the thresholds, and cross-referenced from Common.test.js as the place the painted result is checked. It mounted a hand-built `<Statistic color='warning'/>`. So it proved that role tokens resolve distinctly -- which the roles suite two blocks up already covers -- and would have stayed green with LoadStatistic reverted to `orange`/`red`. The same shape as the column-border test in #609: a claim that outran its measurement. It mounts the real helper at the real thresholds now, and the reversion fails it. The source guards are gone. They scraped function bodies out of files with indexOf slices, covered two helpers, and never executed a branch. `loadRole` and `healthRole` are pure exported functions instead, tested as mappings: either side of both load thresholds, every SMART assessment, lower case (the value comes from smartctl), and the case that matters most -- an unreadable drive must report as unknown, never as healthy. That is also the better answer to the jsdom problem that produced the guards. jsdom rejects `color: var(--danger)` and drops it, so the mapping cannot be read back off a rendered component; extracting it is the honest fix rather than asserting on source text. And the gallery was itself half-migrated: 15 Min. Load became `danger` while 5 Min. Load stayed `orange` -- in the one sample whose caption tells the reader to go and check night and amber, where orange is byte-identical to --text. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
675bd5d620
commit
acd27fe28e
5 changed files with 110 additions and 65 deletions
|
|
@ -1224,15 +1224,24 @@ export function FileIcon({file, disabled = true, size = 48, ...props}) {
|
|||
/>
|
||||
}
|
||||
|
||||
export function LoadStatistic({label, value, cores, ...props}) {
|
||||
// Load is a warning above half the cores, and a problem above three quarters.
|
||||
/**
|
||||
* The severity of a load average: a warning above half the cores, a problem above three
|
||||
* quarters, and nothing to say below that or when the core count is unknown.
|
||||
*
|
||||
* Exported and pure so the branches can be tested as branches. Reading the colour back off
|
||||
* a rendered Statistic cannot do it -- jsdom rejects `color: var(--danger)` as an invalid
|
||||
* declaration and drops it, so every value comes back as the empty string.
|
||||
*/
|
||||
export const loadRole = (value, cores) => {
|
||||
if (!cores) return undefined;
|
||||
const quarter = cores / 4;
|
||||
let color;
|
||||
if (cores && value >= (quarter * 3)) {
|
||||
color = 'danger';
|
||||
} else if (cores && value >= (quarter * 2)) {
|
||||
color = 'warning';
|
||||
}
|
||||
if (value >= quarter * 3) return 'danger';
|
||||
if (value >= quarter * 2) return 'warning';
|
||||
return undefined;
|
||||
};
|
||||
|
||||
export function LoadStatistic({label, value, cores, ...props}) {
|
||||
const color = loadRole(value, cores);
|
||||
return <Statistic
|
||||
label={label}
|
||||
value={value ? parseFloat(value).toFixed(1) : '?'}
|
||||
|
|
|
|||
|
|
@ -1,11 +1,11 @@
|
|||
import fs from 'fs';
|
||||
import path from 'path';
|
||||
import React from 'react';
|
||||
import {act, render, screen, waitFor} from '../test-utils';
|
||||
import userEvent from '@testing-library/user-event';
|
||||
import {
|
||||
contrastingColor, contrastRatio, DirectorySearch, SearchResultsInput, TAG_TEXT_DARK, TAG_TEXT_LIGHT,
|
||||
contrastingColor, contrastRatio, DirectorySearch, loadRole, SearchResultsInput,
|
||||
TAG_TEXT_DARK, TAG_TEXT_LIGHT,
|
||||
} from './Common';
|
||||
import {healthRole} from './admin/ControllerPage';
|
||||
|
||||
// Mock debounce to make tests run faster
|
||||
jest.mock('lodash/debounce', () => jest.fn(fn => {
|
||||
|
|
@ -563,45 +563,62 @@ describe('contrastingColor with shorthand hex', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('LoadStatistic ranks load by role, not by hue', () => {
|
||||
describe('loadRole', () => {
|
||||
/*
|
||||
* A source guard, not a render test, and deliberately so: jsdom's CSS parser REJECTS
|
||||
* `color: var(--danger)` as an invalid declaration and drops it, so every inline colour
|
||||
* reads back as the empty string here. A render test would pass whatever the component
|
||||
* did -- the first version of this asserted `''` for the below-threshold cases and would
|
||||
* have gone on passing with the hues restored.
|
||||
* The branches, tested as branches. This used to be a source guard scraping the
|
||||
* function body out of Common.js, because the obvious render test cannot work: jsdom
|
||||
* REJECTS `color: var(--danger)` as an invalid declaration and drops it, so every inline
|
||||
* colour reads back as the empty string and the assertions passed for that reason alone.
|
||||
* Pulling the mapping out as a pure function is the honest fix.
|
||||
*
|
||||
* What is painted is asserted in ui-layout.cy.js, in a browser, at the thresholds.
|
||||
* Thresholds are halves and three quarters of the core count, so each is asserted either
|
||||
* side of its boundary rather than at one comfortable value.
|
||||
*/
|
||||
it('names roles rather than hues at both thresholds', () => {
|
||||
const source = fs.readFileSync(path.join(__dirname, 'Common.js'), 'utf8');
|
||||
const helper = source.slice(
|
||||
source.indexOf('export function LoadStatistic'),
|
||||
source.indexOf('export function', source.indexOf('export function LoadStatistic') + 10));
|
||||
it('says nothing about a load below half the cores', () => {
|
||||
expect(loadRole(1.9, 4)).toBeUndefined();
|
||||
expect(loadRole(0, 4)).toBeUndefined();
|
||||
});
|
||||
|
||||
expect(helper).toContain("color = 'danger'");
|
||||
expect(helper).toContain("color = 'warning'");
|
||||
expect(helper.match(/'(red|green|amber|yellow|orange)'/)).toBeNull();
|
||||
it('warns from half the cores', () => {
|
||||
expect(loadRole(2.0, 4)).toBe('warning');
|
||||
expect(loadRole(2.9, 4)).toBe('warning');
|
||||
});
|
||||
|
||||
it('calls it danger from three quarters', () => {
|
||||
expect(loadRole(3.0, 4)).toBe('danger');
|
||||
expect(loadRole(99, 4)).toBe('danger');
|
||||
});
|
||||
|
||||
it('says nothing when the core count is unknown', () => {
|
||||
// Both thresholds are guarded on `cores`. Without it every load would read as fine,
|
||||
// which is the safe direction, but worth pinning so it stays deliberate.
|
||||
expect(loadRole(12, undefined)).toBeUndefined();
|
||||
expect(loadRole(12, 0)).toBeUndefined();
|
||||
});
|
||||
|
||||
it('never returns a hue', () => {
|
||||
// The regression this exists for: `orange` is byte-identical to `--text` in night,
|
||||
// so a warning load rendered as an ordinary uncoloured number.
|
||||
[loadRole(1, 4), loadRole(2.5, 4), loadRole(4, 4)].filter(Boolean).forEach(role =>
|
||||
expect(role).not.toMatch(/^(red|green|amber|yellow|orange|blue)$/));
|
||||
});
|
||||
});
|
||||
|
||||
describe('a drive\'s SMART assessment is a severity, not a colour', () => {
|
||||
/*
|
||||
* A source guard: `getHealthColor` is module-private, and exporting it only so a test
|
||||
* can reach it would be shaping the code around the test. What matters is that the
|
||||
* helper stays on the ramp -- all four branches or none, since a helper where PASS and
|
||||
* FAIL rank but WARN is an unranked hue is worse than either choice.
|
||||
*/
|
||||
it('maps every branch to a role', () => {
|
||||
const source = fs.readFileSync(
|
||||
path.join(__dirname, 'admin', 'ControllerPage.js'), 'utf8');
|
||||
const helper = source.slice(
|
||||
source.indexOf('const getHealthColor'),
|
||||
source.indexOf('};', source.indexOf('const getHealthColor')));
|
||||
describe('healthRole', () => {
|
||||
it('maps every SMART assessment to a severity', () => {
|
||||
expect(healthRole('PASS')).toBe('success');
|
||||
expect(healthRole('WARN')).toBe('warning');
|
||||
expect(healthRole('FAIL')).toBe('danger');
|
||||
});
|
||||
|
||||
expect(helper).toContain("'var(--success)'");
|
||||
expect(helper).toContain("'var(--warning)'");
|
||||
expect(helper).toContain("'var(--danger)'");
|
||||
expect(helper.match(/var\(--(red|green|amber|yellow|orange)\)/)).toBeNull();
|
||||
it('is not case sensitive, because the value comes from smartctl', () => {
|
||||
expect(healthRole('pass')).toBe('success');
|
||||
expect(healthRole('Fail')).toBe('danger');
|
||||
});
|
||||
|
||||
it('treats a missing or unrecognised assessment as unknown, not as healthy', () => {
|
||||
// Reporting an unreadable drive as PASS would be the dangerous direction.
|
||||
[undefined, null, '', 'SOMETHING_ELSE'].forEach(value =>
|
||||
expect(healthRole(value)).toBe('neutral'));
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -196,7 +196,7 @@ export function ThemeSamplePage() {
|
|||
<Header as='h5' style={{marginTop: 24}}>Coloured readings, as Status draws them</Header>
|
||||
<StatisticGroup>
|
||||
<Statistic value='0.4' label='1 Min. Load'/>
|
||||
<Statistic value='2.6' label='5 Min. Load' color='orange'/>
|
||||
<Statistic value='2.6' label='5 Min. Load' color='warning'/>
|
||||
<Statistic value='4.1' label='15 Min. Load' color='danger'/>
|
||||
<Statistic value='48' label='Temp C°'/>
|
||||
</StatisticGroup>
|
||||
|
|
|
|||
|
|
@ -605,19 +605,33 @@ function ServicesSection() {
|
|||
|
||||
|
||||
// Helper to get health status color from SMART assessment
|
||||
const getHealthColor = (assessment) => {
|
||||
// Roles, not hues: a drive's assessment is a severity and has to rank as one. All
|
||||
// four branches together -- a helper where PASS and FAIL rank but WARN is an
|
||||
// unranked hue is worse than either choice, and in night `--amber` is byte-identical
|
||||
// to `--yellow` so WARN would stop sitting between them.
|
||||
if (!assessment) return 'var(--neutral)';
|
||||
const upper = assessment.toUpperCase();
|
||||
if (upper === 'PASS') return 'var(--success)';
|
||||
if (upper === 'WARN') return 'var(--warning)';
|
||||
if (upper === 'FAIL') return 'var(--danger)';
|
||||
return 'var(--neutral)';
|
||||
/**
|
||||
* The severity of a SMART assessment.
|
||||
*
|
||||
* Roles, not hues: a drive's health is a severity and has to rank as one. All four
|
||||
* branches together -- a helper where PASS and FAIL rank but WARN is an unranked hue is
|
||||
* worse than either choice, and in night `--amber` is byte-identical to `--yellow`, so WARN
|
||||
* would stop sitting between them.
|
||||
*
|
||||
* Exported and pure so the mapping can be tested as a mapping rather than scraped out of
|
||||
* the source, which is what this needed before it was separated from the CSS wrapper.
|
||||
*/
|
||||
export const healthRole = (assessment) => {
|
||||
if (!assessment) return 'neutral';
|
||||
switch (assessment.toUpperCase()) {
|
||||
case 'PASS':
|
||||
return 'success';
|
||||
case 'WARN':
|
||||
return 'warning';
|
||||
case 'FAIL':
|
||||
return 'danger';
|
||||
default:
|
||||
return 'neutral';
|
||||
}
|
||||
};
|
||||
|
||||
const getHealthColor = (assessment) => `var(--${healthRole(assessment)})`;
|
||||
|
||||
// Helper to format power-on hours
|
||||
const formatPowerOnHours = (hours) => {
|
||||
if (hours === null || hours === undefined) return '-';
|
||||
|
|
|
|||
|
|
@ -4,7 +4,7 @@ import {
|
|||
Message, Placeholder, Statistic, StatisticGroup, Status, TabBar, Table, tabClassName,
|
||||
TextInput,
|
||||
} from './index';
|
||||
import {contrastingColor} from '../Common';
|
||||
import {contrastingColor, LoadStatistic} from '../Common';
|
||||
import {Notifications} from '@mantine/notifications';
|
||||
import {clearToasts, toast} from './toast';
|
||||
import {monochromeThemes, themeNames} from '../../themes/names';
|
||||
|
|
@ -335,26 +335,31 @@ describe('semantic roles read as severity in every theme', () => {
|
|||
|
||||
describe('a load reading ranks itself in the monochrome themes', () => {
|
||||
/*
|
||||
* LoadStatistic is the Status page's CPU load, and the clearest payoff of the roles
|
||||
* outside the library: it used to be `orange` above half the cores and `red` above three
|
||||
* quarters, and in night `--orange` is byte-identical to `--text`, so a machine under
|
||||
* load rendered its warning as an ordinary uncoloured number.
|
||||
* The real `LoadStatistic`, at the real thresholds -- NOT a hand-built `Statistic` with
|
||||
* the roles typed in. The first version of this did that, and it would have stayed green
|
||||
* with `LoadStatistic` reverted to `orange`/`red`: it proved only that role tokens resolve
|
||||
* distinctly, which the roles suite above already covers.
|
||||
*
|
||||
* Only a browser can see this. jsdom rejects `color: var(--warning)` as invalid and
|
||||
* drops it, so the inline colour reads back empty whatever the component did.
|
||||
* This is the Status page's CPU load, and the clearest payoff of the roles outside the
|
||||
* library: `orange` above half the cores was byte-identical to `--text` in night, so a
|
||||
* machine under load rendered its warning as an ordinary uncoloured number.
|
||||
*
|
||||
* Only a browser can see it. jsdom rejects `color: var(--warning)` as invalid and drops
|
||||
* it, so the inline colour reads back empty whatever the component did.
|
||||
*/
|
||||
monochromeThemes.forEach((theme) => {
|
||||
it(`separates fine, warning and danger loads in ${theme}`, () => {
|
||||
cy.mountUI(
|
||||
<Panel>
|
||||
<Statistic value='1.9' label='fine' cores={4} color={undefined}/>
|
||||
<Statistic value='2.6' label='warning' color='warning'/>
|
||||
<Statistic value='4.1' label='danger' color='danger'/>
|
||||
<LoadStatistic label='1 Min. Load' value={1.9} cores={4}/>
|
||||
<LoadStatistic label='5 Min. Load' value={2.6} cores={4}/>
|
||||
<LoadStatistic label='15 Min. Load' value={4.1} cores={4}/>
|
||||
</Panel>,
|
||||
{theme},
|
||||
);
|
||||
|
||||
cy.get('.wrolpi-statistic-value').should(($values) => {
|
||||
expect($values.length, 'three readings').to.equal(3);
|
||||
const [fine, warning, danger] = [...$values]
|
||||
.map(el => toRgb(getComputedStyle(el).color));
|
||||
const panel = toRgb(getComputedStyle(
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue