From cbe8b8a290ae1081a192f661bcc904e22264ba84 Mon Sep 17 00:00:00 2001 From: Tom Ratcliffe Date: Mon, 11 Nov 2024 13:02:46 +0000 Subject: [PATCH] Alerting: Lint and remove unused object properties (#92996) --- eslint.config.js | 5 ++ package.json | 1 + .../alerting/unified/AlertGroups.test.tsx | 2 - .../alerting/unified/MuteTimings.test.tsx | 9 +- .../RuleEditorCloudOnlyAllowed.test.tsx | 4 +- .../unified/RuleEditorGrafanaRules.test.tsx | 4 - .../alerting/unified/RuleList.test.tsx | 11 +-- .../alerting/unified/Silences.test.tsx | 1 - .../export/GrafanaModifyExport.test.tsx | 8 -- .../rule-viewer/RuleViewer.test.tsx | 3 - .../components/rules/RulesGroup.test.tsx | 2 - .../state-history/LokiStateHistory.test.tsx | 1 - .../settings/AlertmanagerConfig.test.tsx | 5 +- .../unified/hooks/useIsRuleEditable.test.tsx | 2 - .../state/AlertmanagerContext.test.tsx | 7 +- yarn.lock | 83 ++++++++++++++++++- 16 files changed, 93 insertions(+), 55 deletions(-) diff --git a/eslint.config.js b/eslint.config.js index 064404a4883..29df2b02563 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -9,6 +9,7 @@ const lodashPlugin = require('eslint-plugin-lodash'); const barrelPlugin = require('eslint-plugin-no-barrel-files'); const reactPlugin = require('eslint-plugin-react'); const testingLibraryPlugin = require('eslint-plugin-testing-library'); +const unicornPlugin = require('eslint-plugin-unicorn'); const grafanaConfig = require('@grafana/eslint-config/flat'); const grafanaPlugin = require('@grafana/eslint-plugin'); @@ -233,11 +234,15 @@ module.exports = [ }, { name: 'grafana/alerting-overrides', + plugins: { + unicorn: unicornPlugin, + }, files: ['public/app/features/alerting/**/*.{ts,tsx,js,jsx}'], rules: { 'dot-notation': 'error', 'prefer-const': 'error', 'react/no-unused-prop-types': 'error', + 'unicorn/no-unused-properties': 'error', }, }, { diff --git a/package.json b/package.json index bb00bd69b33..ad422779c7b 100644 --- a/package.json +++ b/package.json @@ -185,6 +185,7 @@ "eslint-plugin-react": "7.37.2", "eslint-plugin-react-hooks": "5.0.0", "eslint-plugin-testing-library": "^6.3.0", + "eslint-plugin-unicorn": "^56.0.0", "eslint-scope": "^8.1.0", "eslint-webpack-plugin": "4.2.0", "expose-loader": "5.0.0", diff --git a/public/app/features/alerting/unified/AlertGroups.test.tsx b/public/app/features/alerting/unified/AlertGroups.test.tsx index 1243359be61..10782bcedd8 100644 --- a/public/app/features/alerting/unified/AlertGroups.test.tsx +++ b/public/app/features/alerting/unified/AlertGroups.test.tsx @@ -43,11 +43,9 @@ const ui = { group: byTestId('alert-group'), groupCollapseToggle: byTestId('alert-group-collapse-toggle'), groupTable: byTestId('alert-group-table'), - row: byTestId('row'), collapseToggle: byTestId(selectors.components.AlertRules.toggle), silenceButton: byText('Silence'), sourceButton: byText('See alert rule'), - matcherInput: byTestId('search-query-input'), groupByContainer: byTestId('group-by-container'), groupByInput: byRole('combobox', { name: /group by label keys/i }), clearButton: byRole('button', { name: 'Clear filters' }), diff --git a/public/app/features/alerting/unified/MuteTimings.test.tsx b/public/app/features/alerting/unified/MuteTimings.test.tsx index ad600a490f5..0390ab349f7 100644 --- a/public/app/features/alerting/unified/MuteTimings.test.tsx +++ b/public/app/features/alerting/unified/MuteTimings.test.tsx @@ -1,8 +1,8 @@ import { InitialEntry } from 'history'; import { last } from 'lodash'; import { Route, Routes } from 'react-router-dom-v5-compat'; -import { render, screen, userEvent, within } from 'test/test-utils'; -import { byRole, byTestId, byText } from 'testing-library-selector'; +import { render, within, userEvent, screen } from 'test/test-utils'; +import { byTestId } from 'testing-library-selector'; import { config } from '@grafana/runtime'; import { setupMswServer } from 'app/features/alerting/unified/mockApi'; @@ -48,20 +48,15 @@ const dataSources = { }; const ui = { - form: byTestId('mute-timing-form'), nameField: byTestId('mute-timing-name'), startsAt: byTestId('mute-timing-starts-at'), endsAt: byTestId('mute-timing-ends-at'), - addTimeRange: byRole('button', { name: /add another time range/i }), weekdays: byTestId('mute-timing-weekdays'), days: byTestId('mute-timing-days'), months: byTestId('mute-timing-months'), years: byTestId('mute-timing-years'), - - addInterval: byRole('button', { name: /add another time interval/i }), - submitButton: byText(/submit/i), }; const muteTimeInterval: MuteTimeInterval = { diff --git a/public/app/features/alerting/unified/RuleEditorCloudOnlyAllowed.test.tsx b/public/app/features/alerting/unified/RuleEditorCloudOnlyAllowed.test.tsx index c76b8408f2c..37437f4b91e 100644 --- a/public/app/features/alerting/unified/RuleEditorCloudOnlyAllowed.test.tsx +++ b/public/app/features/alerting/unified/RuleEditorCloudOnlyAllowed.test.tsx @@ -10,7 +10,7 @@ import { PromApiFeatures, PromApplication } from 'app/types/unified-alerting-dto import { searchFolders } from '../../manage-dashboards/state/actions'; import { discoverFeatures } from './api/buildInfo'; -import { fetchRulerRules, fetchRulerRulesGroup, fetchRulerRulesNamespace } from './api/ruler'; +import { fetchRulerRulesGroup } from './api/ruler'; import { ExpressionEditorProps } from './components/rule-editor/ExpressionEditor'; import { setupMswServer } from './mockApi'; import { grantUserPermissions, mockDataSource, MockDataSourceSrv } from './mocks'; @@ -119,8 +119,6 @@ const mocks = { api: { discoverFeatures: jest.mocked(discoverFeatures), fetchRulerRulesGroup: jest.mocked(fetchRulerRulesGroup), - fetchRulerRulesNamespace: jest.mocked(fetchRulerRulesNamespace), - fetchRulerRules: jest.mocked(fetchRulerRules), }, }; diff --git a/public/app/features/alerting/unified/RuleEditorGrafanaRules.test.tsx b/public/app/features/alerting/unified/RuleEditorGrafanaRules.test.tsx index 40fd64998b4..9c4ea00134d 100644 --- a/public/app/features/alerting/unified/RuleEditorGrafanaRules.test.tsx +++ b/public/app/features/alerting/unified/RuleEditorGrafanaRules.test.tsx @@ -12,7 +12,6 @@ import { AccessControlAction } from 'app/types'; import { searchFolders } from '../../../../app/features/manage-dashboards/state/actions'; -import { discoverFeatures } from './api/buildInfo'; import { ExpressionEditorProps } from './components/rule-editor/ExpressionEditor'; import { grantUserPermissions, mockDataSource } from './mocks'; import { grafanaRulerGroup, grafanaRulerRule } from './mocks/grafanaRulerApi'; @@ -44,9 +43,6 @@ jest.setTimeout(60 * 1000); const mocks = { getAllDataSources: jest.mocked(config.getAllDataSources), searchFolders: jest.mocked(searchFolders), - api: { - discoverFeatures: jest.mocked(discoverFeatures), - }, }; setupMswServer(); diff --git a/public/app/features/alerting/unified/RuleList.test.tsx b/public/app/features/alerting/unified/RuleList.test.tsx index ef3ec39be48..f42767739e4 100644 --- a/public/app/features/alerting/unified/RuleList.test.tsx +++ b/public/app/features/alerting/unified/RuleList.test.tsx @@ -7,14 +7,7 @@ import { byRole, byTestId, byText } from 'testing-library-selector'; import { PluginExtensionTypes } from '@grafana/data'; import { selectors } from '@grafana/e2e-selectors'; -import { - DataSourceSrv, - getPluginLinkExtensions, - locationService, - setAppEvents, - setDataSourceSrv, - usePluginLinks, -} from '@grafana/runtime'; +import { DataSourceSrv, locationService, setAppEvents, setDataSourceSrv, usePluginLinks } from '@grafana/runtime'; import appEvents from 'app/core/app_events'; import * as ruleActionButtons from 'app/features/alerting/unified/components/rules/RuleActionsButtons'; import { mockUserApi, setupMswServer } from 'app/features/alerting/unified/mockApi'; @@ -71,7 +64,6 @@ setupPluginsExtensionsHook(); const mocks = { getAllDataSourcesMock: jest.mocked(config.getAllDataSources), - getPluginLinkExtensionsMock: jest.mocked(getPluginLinkExtensions), usePluginLinksMock: jest.mocked(usePluginLinks), rulesInSameGroupHaveInvalidForMock: jest.mocked(actions.rulesInSameGroupHaveInvalidFor), @@ -146,7 +138,6 @@ const ui = { more: byRole('button', { name: /More/ }), }, moreActionItems: { - pause: byRole('menuitem', { name: /pause evaluation/i }), resume: byRole('menuitem', { name: /resume evaluation/i }), }, }; diff --git a/public/app/features/alerting/unified/Silences.test.tsx b/public/app/features/alerting/unified/Silences.test.tsx index 416dfe024eb..66fa0bc7a46 100644 --- a/public/app/features/alerting/unified/Silences.test.tsx +++ b/public/app/features/alerting/unified/Silences.test.tsx @@ -65,7 +65,6 @@ const ui = { notExpiredTable: byTestId('not-expired-table'), expiredTable: byTestId('expired-table'), expiredCaret: byText(/expired silences \(/i), - silencesTags: byLabelText(/tags/i), silenceRow: byTestId('row'), silencedAlertCell: byTestId('alerts'), addSilenceButton: byRole('link', { name: /add silence/i }), diff --git a/public/app/features/alerting/unified/components/export/GrafanaModifyExport.test.tsx b/public/app/features/alerting/unified/components/export/GrafanaModifyExport.test.tsx index f64211b329f..e554886aaee 100644 --- a/public/app/features/alerting/unified/components/export/GrafanaModifyExport.test.tsx +++ b/public/app/features/alerting/unified/components/export/GrafanaModifyExport.test.tsx @@ -33,20 +33,12 @@ const ui = { loading: byText('Loading the rule...'), form: { nameInput: byRole('textbox', { name: 'name' }), - folder: byTestId('folder-picker'), - group: byTestId('group-picker'), - annotationKey: (idx: number) => byTestId(`annotation-key-${idx}`), - annotationValue: (idx: number) => byTestId(`annotation-value-${idx}`), - labelKey: (idx: number) => byTestId(`label-key-${idx}`), - labelValue: (idx: number) => byTestId(`label-value-${idx}`), }, exportButton: byRole('button', { name: 'Export' }), exportDrawer: { dialog: byRole('dialog', { name: /Export Group/ }), - jsonTab: byRole('tab', { name: /JSON/ }), yamlTab: byRole('tab', { name: /YAML/ }), editor: byTestId('code-editor'), - loadingSpinner: byTestId('Spinner'), }, }; diff --git a/public/app/features/alerting/unified/components/rule-viewer/RuleViewer.test.tsx b/public/app/features/alerting/unified/components/rule-viewer/RuleViewer.test.tsx index dd7fb820990..6a0fbaf92c4 100644 --- a/public/app/features/alerting/unified/components/rule-viewer/RuleViewer.test.tsx +++ b/public/app/features/alerting/unified/components/rule-viewer/RuleViewer.test.tsx @@ -41,9 +41,6 @@ const ELEMENTS = { details: { pendingPeriod: byText(/Pending period/i), }, - tabs: { - details: byRole('tab', { name: /Details/i }), - }, actions: { edit: byRole('link', { name: 'Edit' }), more: { diff --git a/public/app/features/alerting/unified/components/rules/RulesGroup.test.tsx b/public/app/features/alerting/unified/components/rules/RulesGroup.test.tsx index a868bc87fbd..d79e630ffb5 100644 --- a/public/app/features/alerting/unified/components/rules/RulesGroup.test.tsx +++ b/public/app/features/alerting/unified/components/rules/RulesGroup.test.tsx @@ -59,7 +59,6 @@ const ui = { header: byText('Delete group'), confirmButton: byText('Delete'), }, - moreActionsButton: byRole('button', { name: 'More' }), export: { dialog: byRole('dialog', { name: /Drawer title Export .* rules/ }), jsonTab: byRole('tab', { name: /JSON/ }), @@ -68,7 +67,6 @@ const ui = { copyCodeButton: byRole('button', { name: 'Copy code' }), downloadButton: byRole('button', { name: 'Download' }), }, - loadingSpinner: byTestId('spinner'), }; const server = setupMswServer(); diff --git a/public/app/features/alerting/unified/components/rules/state-history/LokiStateHistory.test.tsx b/public/app/features/alerting/unified/components/rules/state-history/LokiStateHistory.test.tsx index 5b06041b878..415e4f6897b 100644 --- a/public/app/features/alerting/unified/components/rules/state-history/LokiStateHistory.test.tsx +++ b/public/app/features/alerting/unified/components/rules/state-history/LokiStateHistory.test.tsx @@ -76,7 +76,6 @@ window.HTMLElement.prototype.scrollIntoView = jest.fn(); const ui = { loadingIndicator: byText('Loading...'), timestampViewer: byRole('list', { name: 'State history by timestamp' }), - record: byRole('listitem'), noRecords: byText('No state transitions have occurred in the last 30 days'), timelineChart: byTestId('uplot-main-div'), }; diff --git a/public/app/features/alerting/unified/components/settings/AlertmanagerConfig.test.tsx b/public/app/features/alerting/unified/components/settings/AlertmanagerConfig.test.tsx index df4813e52a9..5cc98ea0587 100644 --- a/public/app/features/alerting/unified/components/settings/AlertmanagerConfig.test.tsx +++ b/public/app/features/alerting/unified/components/settings/AlertmanagerConfig.test.tsx @@ -1,9 +1,8 @@ import { waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { render } from 'test/test-utils'; -import { byRole, byTestId } from 'testing-library-selector'; +import { byRole } from 'testing-library-selector'; -import { selectors } from '@grafana/e2e-selectors'; import { AccessControlAction } from 'app/types'; import { setupMswServer } from '../../mockApi'; @@ -37,8 +36,6 @@ const ui = { resetConfirmButton: byRole('button', { name: /Yes, reset configuration/ }), saveButton: byRole('button', { name: /Save/ }), cancelButton: byRole('button', { name: /Cancel/ }), - configInput: byTestId(selectors.components.CodeEditor.container), - readOnlyConfig: byTestId('readonly-config'), }; describe('Alerting Settings', () => { diff --git a/public/app/features/alerting/unified/hooks/useIsRuleEditable.test.tsx b/public/app/features/alerting/unified/hooks/useIsRuleEditable.test.tsx index 77bee61a162..ad3be5526f6 100644 --- a/public/app/features/alerting/unified/hooks/useIsRuleEditable.test.tsx +++ b/public/app/features/alerting/unified/hooks/useIsRuleEditable.test.tsx @@ -9,13 +9,11 @@ import { mockFolder, mockRulerAlertingRule, mockRulerGrafanaRule, mockUnifiedAle import { useFolder } from './useFolder'; import { useIsRuleEditable } from './useIsRuleEditable'; -import { useUnifiedAlertingSelector } from './useUnifiedAlertingSelector'; jest.mock('./useFolder'); const mocks = { useFolder: jest.mocked(useFolder), - useUnifiedAlertingSelector: jest.mocked(useUnifiedAlertingSelector), }; describe('useIsRuleEditable', () => { diff --git a/public/app/features/alerting/unified/state/AlertmanagerContext.test.tsx b/public/app/features/alerting/unified/state/AlertmanagerContext.test.tsx index a6b007f507e..49d0b8c5d79 100644 --- a/public/app/features/alerting/unified/state/AlertmanagerContext.test.tsx +++ b/public/app/features/alerting/unified/state/AlertmanagerContext.test.tsx @@ -11,11 +11,6 @@ import { AlertManagerDataSource, GRAFANA_RULES_SOURCE_NAME } from '../utils/data import { AlertmanagerProvider, isAlertManagerWithConfigAPI, useAlertmanager } from './AlertmanagerContext'; -const grafanaAm: AlertManagerDataSource = { - name: GRAFANA_RULES_SOURCE_NAME, - imgUrl: '', -}; - const externalAmProm: AlertManagerDataSource = { name: 'PrometheusAm', imgUrl: '', @@ -50,7 +45,7 @@ describe('useAlertmanager', () => { ); const { result } = renderHook(() => useAlertmanager(), { wrapper }); - expect(result.current.selectedAlertmanager).toBe(grafanaAm.name); + expect(result.current.selectedAlertmanager).toBe(GRAFANA_RULES_SOURCE_NAME); }); it('Should return alert manager included in the query param when available', () => { diff --git a/yarn.lock b/yarn.lock index 6be10b54af6..2e8a2a8456b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -324,7 +324,7 @@ __metadata: languageName: node linkType: hard -"@babel/helper-validator-identifier@npm:^7.25.9": +"@babel/helper-validator-identifier@npm:^7.24.7, @babel/helper-validator-identifier@npm:^7.25.9": version: 7.25.9 resolution: "@babel/helper-validator-identifier@npm:7.25.9" checksum: 10/3f9b649be0c2fd457fa1957b694b4e69532a668866b8a0d81eabfa34ba16dbf3107b39e0e7144c55c3c652bf773ec816af8df4a61273a2bb4eb3145ca9cf478e @@ -13058,6 +13058,13 @@ __metadata: languageName: node linkType: hard +"builtin-modules@npm:^3.3.0": + version: 3.3.0 + resolution: "builtin-modules@npm:3.3.0" + checksum: 10/62e063ab40c0c1efccbfa9ffa31873e4f9d57408cb396a2649981a0ecbce56aabc93c28feaccbc5658c95aab2703ad1d11980e62ec2e5e72637404e1eb60f39e + languageName: node + linkType: hard + "bundle-name@npm:^4.1.0": version: 4.1.0 resolution: "bundle-name@npm:4.1.0" @@ -13539,6 +13546,15 @@ __metadata: languageName: node linkType: hard +"clean-regexp@npm:^1.0.0": + version: 1.0.0 + resolution: "clean-regexp@npm:1.0.0" + dependencies: + escape-string-regexp: "npm:^1.0.5" + checksum: 10/0b1ce281b07da2463c6882ea2e8409119b6cabbd9f687cdbdcee942c45b2b9049a2084f7b5f228c63ef9f21e722963ae0bfe56a735dbdbdd92512867625a7e40 + languageName: node + linkType: hard + "clean-stack@npm:^2.0.0": version: 2.2.0 resolution: "clean-stack@npm:2.2.0" @@ -16995,6 +17011,32 @@ __metadata: languageName: node linkType: hard +"eslint-plugin-unicorn@npm:^56.0.0": + version: 56.0.0 + resolution: "eslint-plugin-unicorn@npm:56.0.0" + dependencies: + "@babel/helper-validator-identifier": "npm:^7.24.7" + "@eslint-community/eslint-utils": "npm:^4.4.0" + ci-info: "npm:^4.0.0" + clean-regexp: "npm:^1.0.0" + core-js-compat: "npm:^3.38.1" + esquery: "npm:^1.6.0" + globals: "npm:^15.9.0" + indent-string: "npm:^4.0.0" + is-builtin-module: "npm:^3.2.1" + jsesc: "npm:^3.0.2" + pluralize: "npm:^8.0.0" + read-pkg-up: "npm:^7.0.1" + regexp-tree: "npm:^0.1.27" + regjsparser: "npm:^0.10.0" + semver: "npm:^7.6.3" + strip-indent: "npm:^3.0.0" + peerDependencies: + eslint: ">=8.56.0" + checksum: 10/142c66c65b2fd53136727a434b0fc77e9a9f9614aebe09330aeab83b021c842c3a5f9dafe3130c0f39fbd3562e91aadcc55a9de4312639e70fe7efb475cd358e + languageName: node + linkType: hard + "eslint-scope@npm:5.1.1, eslint-scope@npm:^5.1.1": version: 5.1.1 resolution: "eslint-scope@npm:5.1.1" @@ -18584,6 +18626,13 @@ __metadata: languageName: node linkType: hard +"globals@npm:^15.9.0": + version: 15.12.0 + resolution: "globals@npm:15.12.0" + checksum: 10/07cac4ee7cc9befa7894be9b4d1a57f46eeedf9065939f39ffb875009394908eb7bac84147712cfd4bbabab5abc7ab98fc3a6d0fd881f9548fffa10ba2e4bf67 + languageName: node + linkType: hard + "globalthis@npm:^1.0.3, globalthis@npm:^1.0.4": version: 1.0.4 resolution: "globalthis@npm:1.0.4" @@ -18856,6 +18905,7 @@ __metadata: eslint-plugin-react: "npm:7.37.2" eslint-plugin-react-hooks: "npm:5.0.0" eslint-plugin-testing-library: "npm:^6.3.0" + eslint-plugin-unicorn: "npm:^56.0.0" eslint-scope: "npm:^8.1.0" eslint-webpack-plugin: "npm:4.2.0" expose-loader: "npm:5.0.0" @@ -20175,6 +20225,15 @@ __metadata: languageName: node linkType: hard +"is-builtin-module@npm:^3.2.1": + version: 3.2.1 + resolution: "is-builtin-module@npm:3.2.1" + dependencies: + builtin-modules: "npm:^3.3.0" + checksum: 10/e8f0ffc19a98240bda9c7ada84d846486365af88d14616e737d280d378695c8c448a621dcafc8332dbf0fcd0a17b0763b845400709963fa9151ddffece90ae88 + languageName: node + linkType: hard + "is-callable@npm:^1.1.3, is-callable@npm:^1.1.4, is-callable@npm:^1.2.7": version: 1.2.7 resolution: "is-callable@npm:1.2.7" @@ -21580,7 +21639,7 @@ __metadata: languageName: node linkType: hard -"jsesc@npm:^0.5.0": +"jsesc@npm:^0.5.0, jsesc@npm:~0.5.0": version: 0.5.0 resolution: "jsesc@npm:0.5.0" bin: @@ -28196,6 +28255,15 @@ __metadata: languageName: node linkType: hard +"regexp-tree@npm:^0.1.27": + version: 0.1.27 + resolution: "regexp-tree@npm:0.1.27" + bin: + regexp-tree: bin/regexp-tree + checksum: 10/08c70c8adb5a0d4af1061bf9eb05d3b6e1d948c433d6b7008e4b5eb12a49429c2d6ca8e9106339a432aa0d07bd6e1bccc638d8f4ab0d045f3adad22182b300a2 + languageName: node + linkType: hard + "regexp.prototype.flags@npm:^1.5.2": version: 1.5.2 resolution: "regexp.prototype.flags@npm:1.5.2" @@ -28229,6 +28297,17 @@ __metadata: languageName: node linkType: hard +"regjsparser@npm:^0.10.0": + version: 0.10.0 + resolution: "regjsparser@npm:0.10.0" + dependencies: + jsesc: "npm:~0.5.0" + bin: + regjsparser: bin/parser + checksum: 10/06f7f0e59598de20769ce5637bbd8879387f67c0eeb8ccc8857331c623332718c25d8d20bd74df210bf636dde061474e8bd365cf73af20470f0b3cb42cd42019 + languageName: node + linkType: hard + "regjsparser@npm:^0.11.0": version: 0.11.0 resolution: "regjsparser@npm:0.11.0"