chore: fix test noise (#1057)

* chore: fix test noise

* fix: add testing trophy link

docs/development/testing.md

@@ -0,0 +1,15 @@
+# Testing strategy
+
+We use `jest` with `react-testing-library` for testing our application. Our aim is to have a [trophy-shaped testing strategy](https://kentcdodds.com/blog/write-tests), with a large number of integration tests and a smaller number of unit and e2e tests.
+
+The general philosophy is aiming for the right balance of confidence in core user journeys whilst keeping our tests easy to maintain and fast to run and iterate upon. Unit tests are too closely tied to implementation details and can create friction to changing the fundamentals, e2e tests can take a long time to run and be difficult to set up an easy to reproduce environment. Integration tests are by no means perfect but they are a good compromise.
+
+We will continually review our testing strategy and adjust as necessary.
+
+## Silenced test errors
+
+Because we try to emulate so much of the browser in our integration tests, alongside having a dependency on `grafana/grafana` and associated libraries we end up with somewhat noisy tests that generate errors and warnings that aren't very useful.
+
+We have a file [`src/test/silenceErrors.ts`](../../src/test/silenceErrors.ts) that silences some of the more common errors that we see in our tests. This is a bit of a blunt instrument and has an unfortunate side effect of making the stack trace for useful errors in the test difficult to use, but it's better than having to wade through a sea of red text to find the actual test failures in the first place.
+
+If you do need an accurate stack trace, temporarily comment out all of the `silenceErrors.ts` file and re-run the individual test(s) that you're interested in.

src/components/CheckEditor/transformations/toFormValues.http.ts

@@ -13,7 +13,9 @@ import {
 import {
   getBaseFormValuesFromCheck,
   getTlsConfigFormValues,
+  predefinedAlertsToFormValues,
 } from 'components/CheckEditor/transformations/toFormValues.utils';
+import { HTTP_PREDEFINED_ALERTS } from 'components/CheckForm/AlertsPerCheck/AlertsPerCheck.constants';
 import { FALLBACK_CHECK_HTTP, HTTP_COMPRESSION_ALGO_OPTIONS } from 'components/constants';
 
 export function getHTTPCheckFormValues(check: HTTPCheck): CheckFormValuesHttp {
@@ -25,6 +27,10 @@ export function getHTTPCheckFormValues(check: HTTPCheck): CheckFormValuesHttp {
     settings: {
       http: getHttpSettingsForm(check.settings),
     },
+    alerts: {
+      ...base.alerts,
+      ...predefinedAlertsToFormValues(HTTP_PREDEFINED_ALERTS),
+    },
   };
 }
 

src/components/CheckEditor/transformations/toFormValues.ping.ts

@@ -1,5 +1,9 @@
 import { CheckFormValuesPing, CheckType, PingCheck, PingSettingsFormValues } from 'types';
-import { getBaseFormValuesFromCheck } from 'components/CheckEditor/transformations/toFormValues.utils';
+import {
+  getBaseFormValuesFromCheck,
+  predefinedAlertsToFormValues,
+} from 'components/CheckEditor/transformations/toFormValues.utils';
+import { PING_PREDEFINED_ALERTS } from 'components/CheckForm/AlertsPerCheck/AlertsPerCheck.constants';
 import { FALLBACK_CHECK_PING } from 'components/constants';
 
 export function getPingCheckFormValues(check: PingCheck): CheckFormValuesPing {
@@ -11,6 +15,10 @@ export function getPingCheckFormValues(check: PingCheck): CheckFormValuesPing {
     settings: {
       ping: getPingSettingsFormValues(check.settings),
     },
+    alerts: {
+      ...base.alerts,
+      ...predefinedAlertsToFormValues(PING_PREDEFINED_ALERTS),
+    },
   };
 }
 

src/components/CheckEditor/transformations/toFormValues.utils.ts

@@ -2,6 +2,10 @@ import { SelectableValue } from '@grafana/data';
 
 import { Check, CheckFormValues, TLSConfig } from 'types';
 import { fromBase64 } from 'utils';
+import {
+  GLOBAL_PREDEFINED_ALERTS,
+  PredefinedAlertInterface,
+} from 'components/CheckForm/AlertsPerCheck/AlertsPerCheck.constants';
 
 export function selectableValueFrom<T>(value: T, label?: string): SelectableValue<T> {
   const labelValue = String(value);
@@ -49,5 +53,18 @@ export function getBaseFormValuesFromCheck(check: Check): Omit<CheckFormValues,
     probes: check.probes,
     target: check.target,
     timeout,
+    alerts: predefinedAlertsToFormValues(GLOBAL_PREDEFINED_ALERTS),
   };
 }
+
+export function predefinedAlertsToFormValues(predefinedAlerts: PredefinedAlertInterface[]) {
+  return Object.values(predefinedAlerts).reduce((acc, alert) => {
+    return {
+      ...acc,
+      [alert.type]: {
+        threshold: alert.default,
+        isSelected: false,
+      },
+    };
+  }, {});
+}

src/components/CheckForm/AlertsPerCheck/AlertItem.tsx

@@ -20,14 +20,13 @@ export const AlertItem = ({
 }) => {
   const styles = useStyles2(getStyles);
 
-  const { control, formState, getValues } = useFormContext<CheckFormValues>();
+  const { control, formState } = useFormContext<CheckFormValues>();
   const { isFormDisabled } = useCheckFormContext();
 
   const handleToggleAlert = (type: CheckAlertType) => {
     onSelectionChange(type);
   };
 
-  const threshold: number = getValues(`alerts.${alert.type}.threshold`);
   const thresholdError = formState.errors?.alerts?.[alert.type]?.threshold?.message;
 
   return (
@@ -48,22 +47,24 @@ export const AlertItem = ({
           <Controller
             name={`alerts.${alert.type}.threshold`}
             control={control}
-            render={({ field }) => (
-              <Input
-                aria-disabled={!selected}
-                suffix={alert.unit}
-                type="number"
-                step="any"
-                id={`alert-threshold-${alert.type}`}
-                value={field.value !== undefined ? field.value : threshold}
-                onChange={(e) => {
-                  const value = e.currentTarget.value;
-                  return field.onChange(value !== '' ? Number(value) : undefined);
-                }}
-                width={10}
-                disabled={!selected || isFormDisabled}
-              />
-            )}
+            render={({ field }) => {
+              return (
+                <Input
+                  {...field}
+                  aria-disabled={!selected}
+                  suffix={alert.unit}
+                  type="number"
+                  step="any"
+                  id={`alert-threshold-${alert.type}`}
+                  onChange={(e) => {
+                    const value = e.currentTarget.value;
+                    return field.onChange(value !== '' ? Number(value) : '');
+                  }}
+                  width={10}
+                  disabled={!selected || isFormDisabled}
+                />
+              );
+            }}
           />
         </Field>
       </div>

src/components/CheckForm/AlertsPerCheck/AlertsPerCheck.constants.tsx

@@ -8,7 +8,7 @@ export interface PredefinedAlertInterface {
   default?: number;
 }
 
-const GLOBAL_PREDEFINED_ALERTS: PredefinedAlertInterface[] = [
+export const GLOBAL_PREDEFINED_ALERTS: PredefinedAlertInterface[] = [
   {
     type: CheckAlertType.ProbeFailedExecutionsTooHigh,
     name: 'Probe Failed Executions Too High',
@@ -18,7 +18,7 @@ const GLOBAL_PREDEFINED_ALERTS: PredefinedAlertInterface[] = [
   },
 ];
 
-const HTTP_PREDEFINED_ALERTS: PredefinedAlertInterface[] = [
+export const HTTP_PREDEFINED_ALERTS: PredefinedAlertInterface[] = [
   {
     type: CheckAlertType.HTTPRequestDurationTooHighP50,
     name: 'HTTP Request Duration Too High (P50)',
@@ -56,7 +56,7 @@ const HTTP_PREDEFINED_ALERTS: PredefinedAlertInterface[] = [
   },
 ];
 
-const PING_PREDEFINED_ALERTS: PredefinedAlertInterface[] = [
+export const PING_PREDEFINED_ALERTS: PredefinedAlertInterface[] = [
   {
     type: CheckAlertType.PingICMPDurationTooHighP50,
     name: 'Ping ICMP Duration Too High (P50)',

src/components/CheckForm/AlertsPerCheck/AlertsPerCheck.tsx

@@ -6,7 +6,6 @@ import { css } from '@emotion/css';
 
 import { CheckAlertFormValues, CheckAlertType, CheckFormValues, CheckStatus } from 'types';
 import { useListAlertsForCheck } from 'data/useCheckAlerts';
-import { getAlertCheckFormValues } from 'components/CheckEditor/transformations/toFormValues.alerts';
 import { NewStatusBadge } from 'components/NewStatusBadge';
 
 import { AlertsList } from './AlertsList';
@@ -30,9 +29,30 @@ export const AlertsPerCheck = ({ onInitAlerts }: AlertsPerCheckProps) => {
     if (!checkAlerts) {
       return;
     }
-    const formAlerts = getAlertCheckFormValues(checkAlerts);
-    setValue(`alerts`, formAlerts, { shouldDirty: false });
-    onInitAlerts(formAlerts);
+
+    onInitAlerts(
+      checkAlerts.reduce((acc, alert) => {
+        return {
+          ...acc,
+          [alert.name]: {
+            threshold: alert.threshold,
+            isSelected: true,
+          },
+        };
+      }, {})
+    );
+
+    checkAlerts.forEach((alert) => {
+      setValue(
+        `alerts.${alert.name}`,
+        // @ts-expect-error
+        {
+          threshold: alert.threshold,
+          isSelected: true,
+        },
+        { shouldDirty: false }
+      );
+    });
   }, [checkAlerts, setValue, onInitAlerts]);
 
   const groupedByCategory = useMemo(

src/components/CheckForm/CheckForm.tsx

@@ -174,7 +174,13 @@ export const CheckForm = ({ check, disabled }: CheckFormProps) => {
 
   const handleInitAlerts = useCallback(
     (alerts: Partial<Record<CheckAlertType, CheckAlertFormValues>>) => {
-      const newDefaults = { ...defaultValues, alerts };
+      const newDefaults = {
+        ...defaultValues,
+        alerts: {
+          ...defaultValues.alerts,
+          ...alerts,
+        },
+      };
       setFormDefaultValues(newDefaults);
     },
     [defaultValues, setFormDefaultValues]

src/components/CheckForm/FormLayout/FormLayout.tsx

@@ -148,7 +148,7 @@ export const FormLayout = <T extends FieldValues>({
                   </Button>
                 )}
                 <Button
-                  data-testId={DataTestIds.CHECK_FORM_SUBMIT_BUTTON}
+                  data-testid={DataTestIds.CHECK_FORM_SUBMIT_BUTTON}
                   disabled={disableSubmit}
                   key="submit"
                   type="submit"

src/components/NameValueInput/NameValueInput.tsx

@@ -130,8 +130,8 @@ export const NameValueInput = ({ name, disabled, limit, label, ...rest }: Props)
 };
 
 const getStyles = (theme: GrafanaTheme2) => ({
-  addButton: css({ 'margin-top': theme.spacing(1) }),
-  field: css({ 'margin-bottom': 0, 'margin-top': 0 }),
+  addButton: css({ marginTop: theme.spacing(1) }),
+  field: css({ marginBottom: 0, marginTop: 0 }),
   stack: css({
     display: `flex`,
     gap: theme.spacing(1),

src/data/useCheckAlerts.ts

@@ -8,6 +8,7 @@ import { SMDataSource } from 'datasource/DataSource';
 import { CheckAlertsResponse } from 'datasource/responses.types';
 import { queryClient } from 'data/queryClient';
 import { useSMDS } from 'hooks/useSMDS';
+
 export const queryKeys: Record<'listAlertsForCheck', QueryKey> = {
   listAlertsForCheck: ['alertsForCheck'],
 };
@@ -17,6 +18,7 @@ const alertsForCheckQuery = (api: SMDataSource, checkId?: number) => {
     enabled: Boolean(checkId),
     queryKey: [...queryKeys.listAlertsForCheck, checkId],
     queryFn: () => api.listAlertsForCheck(checkId!),
+    select: (data: CheckAlertsResponse) => data.alerts,
   };
 };
 

src/page/ConfigPageLayout/ConfigContent.tsx

@@ -53,11 +53,12 @@ ConfigContent.Section = function ConfigContentSection({
   className,
 }: PropsWithChildren<{ title?: ReactNode; className?: string }>) {
   const Container = className ? 'div' : Fragment;
+  const props = className ? { className } : {};
 
   return (
     <Box marginBottom={4} element="section">
       {!!title && <h3>{title}</h3>}
-      <Container className={className}>{children}</Container>
+      <Container {...props}>{children}</Container>
     </Box>
   );
 };

src/page/NewCheck/__tests__/ApiEndPointChecks/PingCheck/4-alerting.payload.test.tsx

@@ -68,7 +68,10 @@ describe(`PingCheck - Section 4 (Alerting) payload`, () => {
 
     expect(alertsBody).toEqual({
       alerts: [
-        { name: 'ProbeFailedExecutionsTooHigh', threshold: 0.1 },
+        {
+          name: 'ProbeFailedExecutionsTooHigh',
+          threshold: 0.1,
+        },
         {
           name: 'PingICMPDurationTooHighP50',
           threshold: 1,

src/test/silenceErrors.ts

@@ -1,9 +1,9 @@
-// silence container query errors
 const originalConsoleError = console.error;
 
 const ignoreErrorsList = [
-  `Could not parse CSS stylesheet`,
-  `Using kebab-case for css properties in objects is not supported.`,
+  `Could not parse CSS stylesheet`, // silence container query errors
+  `Warning: Received \`%s\` for a non-boolean attribute \`%s\``, // should be fixed upstream
+  `Warning: validateDOMNesting(...): %s cannot appear as a descendant of <%s>.%s`, // probecard - card.meta in grafana/grafana is a paragraph tag
 ];
 
 beforeAll(() => {