diff --git a/frontend/src/__tests__/dashboard.test.ts b/frontend/src/__tests__/dashboard.test.ts index 6167608a8..5f50d1fe1 100644 --- a/frontend/src/__tests__/dashboard.test.ts +++ b/frontend/src/__tests__/dashboard.test.ts @@ -2014,7 +2014,8 @@ describe('Dashboard Module', () => { // ec2: 40 current + 60 lowest option + 200 upside = 300; rds: 50. Total 350. expect(label).toContain('2 services'); expect(label).toContain('$350 in total'); - expect(label).toContain('Largest: ec2 at $300'); + // #1649: the text alternative uses the same names as the axis. + expect(label).toContain('Largest: EC2 at $300'); }); test('lists every layer value per service in a "View as table" disclosure', () => { @@ -2026,9 +2027,10 @@ describe('Dashboard Module', () => { expect(document.querySelector('#savings-by-service-table summary')?.textContent).toBe('View as table'); const headers = Array.from(document.querySelectorAll('#savings-by-service-table thead th')).map((th) => th.textContent); expect(headers).toEqual(['Service', 'Current / Committed', 'Lowest option', 'Upside', 'Total']); + // #1649: display names, matching the chart axis. expect(tableRows()).toEqual([ - ['ec2', '$40', '$60', '$200', '$300'], - ['rds', '$0', '$50', '$0', '$50'], + ['EC2', '$40', '$60', '$200', '$300'], + ['RDS', '$0', '$50', '$0', '$50'], ]); }); diff --git a/frontend/src/__tests__/plans-column-filters.test.ts b/frontend/src/__tests__/plans-column-filters.test.ts index 4d749d07c..e385833cd 100644 --- a/frontend/src/__tests__/plans-column-filters.test.ts +++ b/frontend/src/__tests__/plans-column-filters.test.ts @@ -233,7 +233,8 @@ describe('Planned Purchases column filters (issue #166 follow-up)', () => { const survivor = document.querySelector( '#planned-purchases-list tbody tr.planned-purchase-row', ); - expect(survivor?.innerHTML).toContain('rds'); + // #1683: the Service cell shows the display name. + expect(survivor?.innerHTML).toContain('RDS'); }); test('invalid numeric expression surfaces the lib error inline (no filter applied)', async () => { diff --git a/frontend/src/__tests__/plans-service-ids.test.ts b/frontend/src/__tests__/plans-service-ids.test.ts index 88440a284..20a29b812 100644 --- a/frontend/src/__tests__/plans-service-ids.test.ts +++ b/frontend/src/__tests__/plans-service-ids.test.ts @@ -148,6 +148,21 @@ describe('plan modal service ids (#608)', () => { expect(showToast).not.toHaveBeenCalled(); }); + // The card shows display names (#1683); the edit flow must still select the + // option by slug. Before, the card's "Compute SP" label was also used as the + // id, so a Savings Plans plan opened with "no longer supported". + test.each([ + ['aws', 'ec2'], + ['aws', 'rds'], + ['aws', 'savings-plans-compute'], + ])('edit flow of a saved %s plan with service %s selects its option without an error', async (provider, saved) => { + await editSavedPlan(provider, saved); + + const select = document.getElementById('plan-service') as HTMLSelectElement; + expect(select.selectedOptions[0]!.value).toBe(saved); + expect(showToast).not.toHaveBeenCalled(); + }); + test.each([ ['azure', 'cosmosdb'], ['gcp', 'cloudstorage'], diff --git a/frontend/src/__tests__/plans.test.ts b/frontend/src/__tests__/plans.test.ts index d6f038821..4e6058d5e 100644 --- a/frontend/src/__tests__/plans.test.ts +++ b/frontend/src/__tests__/plans.test.ts @@ -248,7 +248,8 @@ describe('Plans Module', () => { const list = document.getElementById('plans-list'); expect(list?.innerHTML).toContain('Test Plan'); - expect(list?.innerHTML).toContain('ec2'); + // #1683: the Service field shows the display name. + expect(list?.innerHTML).toContain('EC2'); }); test.each([ @@ -341,8 +342,11 @@ describe('Plans Module', () => { await loadPlans(); const list = document.getElementById('plans-list'); - // The Service field shows "ec2" — the slug pass-through case. - expect(list?.innerHTML).toContain('ec2'); + // #1683: the Service field shows the display name, not the slug. + const values = Array.from(list?.querySelectorAll('.plan-detail') ?? []) + .filter((d) => d.querySelector('.plan-detail-label')?.textContent === 'Service') + .map((d) => d.querySelector('.plan-detail-value')?.textContent); + expect(values).toEqual(['EC2']); }); test('shows empty message when no plans', async () => { @@ -730,6 +734,26 @@ describe('Plans Module', () => { expect(container?.innerHTML).toContain('us-east-1'); }); + // #1683: the Service cell shows the display name, not the slug. + test('shows the service display name in the planned purchases table', async () => { + (api.getPlans as jest.Mock).mockResolvedValue({ plans: [] }); + (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ + purchases: [{ + id: 'purchase-rds', plan_id: 'plan-1', plan_name: 'RDS plan', scheduled_date: '2024-02-20', + provider: 'aws', service: 'rds', resource_type: 'db.t3.micro', region: 'us-east-1', count: 1, + term: 1, payment: 'no-upfront', upfront_cost: 0, estimated_savings: 5, status: 'pending', + step_number: 1, total_steps: 1, + }], + }); + + await loadPlans(); + + const cells = Array.from(document.querySelectorAll('#planned-purchases-list tbody td')) + .map((td) => td.textContent?.trim()); + expect(cells).toContain('RDS'); + expect(cells).not.toContain('rds'); + }); + test('labels a step with no recommendations instead of rendering zeros (#609)', async () => { (api.getPlans as jest.Mock).mockResolvedValue({ plans: [] }); (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ diff --git a/frontend/src/__tests__/recommendations.test.ts b/frontend/src/__tests__/recommendations.test.ts index 8a2cd3f2a..01d293536 100644 --- a/frontend/src/__tests__/recommendations.test.ts +++ b/frontend/src/__tests__/recommendations.test.ts @@ -418,7 +418,8 @@ describe('Recommendations Module', () => { await loadRecommendations(); expect(api.getRecommendations).toHaveBeenLastCalledWith(expect.objectContaining({ service: 'ec2' })); - expect(scopeLine()).toBe('Filtered: 2 recommendations (Service: ec2)'); + // #1683: the pushed-down service filter is named like the Service column. + expect(scopeLine()).toBe('Filtered: 2 recommendations (Service: EC2)'); // A further client-side filter is counted, not named. (state.getRecommendationsColumnFilters as jest.Mock).mockReturnValue({ @@ -426,7 +427,7 @@ describe('Recommendations Module', () => { term: { kind: 'set', values: ['1'] }, }); document.querySelector('th[data-sort="service"]')?.click(); - expect(scopeLine()).toBe('Filtered: 2 recommendations (Service: ec2, +1 more filter)'); + expect(scopeLine()).toBe('Filtered: 2 recommendations (Service: EC2, +1 more filter)'); } finally { // Reload unfiltered so the pushed-down scope doesn't leak into later tests. (state.getRecommendationsColumnFilters as jest.Mock).mockReturnValue({}); @@ -2854,6 +2855,16 @@ describe('Bundle B: column header filter triggers', () => { expect(values.sort()).toEqual(['aws', 'azure']); }); + // #1683: the Service filter lists display names but still filters on slugs. + test('service popover shows display names and keeps slug values', async () => { + await loadRecommendations(); + document.querySelector('th .column-filter-btn[data-column="service"]')?.click(); + const items = Array.from( + document.querySelectorAll('.column-filter-popover .column-filter-item'), + ).map((label) => [label.querySelector('input')?.dataset['value'], label.textContent]); + expect(items).toContainEqual(['ec2', 'EC2']); + }); + test('service popover includes SageMaker when the loaded rec set has it', async () => { const sagemakerRecs = [ ...sampleRecs, @@ -3294,11 +3305,12 @@ describe('Bundle B: column header filter triggers', () => { test('Azure SP rows (service="savingsplans") appear in the rendered table', async () => { await loadRecommendations(); - // The table should contain a row with service badge "savingsplans". + // The row's service badge shows the display name (#1683), not the slug. const serviceBadges = Array.from( document.querySelectorAll('td .service-badge'), ).map((el) => el.textContent ?? ''); - expect(serviceBadges).toContain('savingsplans'); + expect(serviceBadges).toContain('Savings Plans'); + expect(serviceBadges).not.toContain('savingsplans'); }); test('service popover renders All Savings Plans toggle when Azure SP + AWS SP slugs are present', async () => { diff --git a/frontend/src/__tests__/service-labels.test.ts b/frontend/src/__tests__/service-labels.test.ts new file mode 100644 index 000000000..d3f11205b --- /dev/null +++ b/frontend/src/__tests__/service-labels.test.ts @@ -0,0 +1,25 @@ +import { serviceLabel } from '../lib/service-labels'; + +// Issues #1649 / #1683: one display name per service slug for every surface. +describe('serviceLabel', () => { + test.each([ + ['ec2', 'EC2'], + ['rds', 'RDS'], + ['relational-db', 'Relational DB'], + ['data-warehouse', 'Data Warehouse'], + ['compute', 'Compute'], + ['savings-plans-compute', 'Compute Savings Plans'], + ['savingsplans', 'Savings Plans'], + ])('%s -> %s', (slug, label) => { + expect(serviceLabel(slug)).toBe(label); + }); + + test('compact form abbreviates Savings Plans', () => { + expect(serviceLabel('savings-plans-ec2instance', true)).toBe('EC2 Instance SP'); + expect(serviceLabel('rds', true)).toBe('RDS'); + }); + + test('unknown slugs pass through unchanged', () => { + expect(serviceLabel('some-new-service')).toBe('some-new-service'); + }); +}); diff --git a/frontend/src/dashboard.ts b/frontend/src/dashboard.ts index 12fe45c1d..5a1fd274f 100644 --- a/frontend/src/dashboard.ts +++ b/frontend/src/dashboard.ts @@ -15,6 +15,7 @@ import { canAccess } from './permissions'; import { groupRecsByCell, pageLevelRange, formatSavingsRange, triggerAutoRefreshIfStale, startRecommendationsRefresh } from './recommendations'; import { getRecommendationsFreshness } from './api/recommendations'; import { buildFirstRunEmptyState, isScopeFiltered } from './first-run-empty-state'; +import { serviceLabel } from './lib/service-labels'; import { showSkeletonTiles, showSkeletonBlock, teardownSkeleton } from './lib/skeleton'; import { buildErrorCallout } from './lib/error-callout'; import { filterByEnabledProviders } from './lib/enabled-providers'; @@ -50,33 +51,6 @@ function segmentGap() { return { borderColor: surfaceColor(), borderWidth: { top: 0, right: 2, bottom: 0, left: 0 } } as const; } -// Display names for the per-service chart's axis and tooltips. The chart -// groups by service slug across providers, so shared slugs (compute, cache, -// relational-db) get a provider-neutral name. Unknown slugs pass through. -const SERVICE_CHART_LABELS: Readonly> = { - ec2: 'EC2', - rds: 'RDS', - elasticache: 'ElastiCache', - opensearch: 'OpenSearch', - memorydb: 'MemoryDB', - redshift: 'Redshift', - 'savings-plans-compute': 'Compute Savings Plans', - 'savings-plans-ec2instance': 'EC2 Instance Savings Plans', - 'savings-plans-sagemaker': 'SageMaker Savings Plans', - 'savings-plans-database': 'Database Savings Plans', - compute: 'Compute', - 'relational-db': 'Relational DB', - cache: 'Cache', - search: 'Search', - nosql: 'NoSQL', -}; - -export function serviceChartLabel(slug: string, compact = false): string { - const label = SERVICE_CHART_LABELS[slug] ?? slug; - // Narrow charts abbreviate "Savings Plans" the way plan cards do ("Compute SP"). - return compact ? label.replace(/ Savings Plans$/, ' SP') : label; -} - // Below this chart width the category labels use the compact names. const SERVICE_CHART_COMPACT_PX = 560; @@ -1064,12 +1038,12 @@ export function renderSavingsByService( const top = positive[0]!; setChartSummary(canvas, `Stacked bar chart of monthly savings for ${positive.length} service${positive.length === 1 ? '' : 's'}, ` - + `${formatCurrency(totalSavings)} in total. Largest: ${top.svc} at ${formatCurrency(top.visibleTotal)}. ` + + `${formatCurrency(totalSavings)} in total. Largest: ${serviceLabel(top.svc)} at ${formatCurrency(top.visibleTotal)}. ` + 'Layers: Current / Committed, Lowest option, Upside. Values are in the table below the chart.'); renderChartTable(canvas, SAVINGS_BY_SERVICE_TABLE_ID, { caption: 'Potential savings range per service (monthly)', columns: ['Service', 'Current / Committed', 'Lowest option', 'Upside', 'Total'], - rows: positive.map(e => [e.svc, formatCurrency(e.current), formatCurrency(e.lowestOption), + rows: positive.map(e => [serviceLabel(e.svc), formatCurrency(e.current), formatCurrency(e.lowestOption), formatCurrency(e.upside), formatCurrency(e.visibleTotal)]), }); @@ -1127,7 +1101,7 @@ export function renderSavingsByService( }, tooltip: { callbacks: { - title: (items) => serviceChartLabel(items[0]?.label ?? ''), + title: (items) => serviceLabel(items[0]?.label ?? ''), label: (ctx) => { const svc = ctx.label ?? ''; const s = stats.get(svc); @@ -1148,7 +1122,7 @@ export function renderSavingsByService( const total = current + lowestOption + upside; const pct = totalSavings > 0 ? ((total / totalSavings) * 100).toFixed(1) : '0.0'; const lines = [ - `Service: ${serviceChartLabel(svc)}`, + `Service: ${serviceLabel(svc)}`, `Total: ${formatCurrency(total)} (${pct}% of all services)`, `Current / Committed: ${formatCurrency(current)}`, `Lowest option: ${formatCurrency(lowestOption)}`, @@ -1179,7 +1153,7 @@ export function renderSavingsByService( ticks: { autoSkip: false, callback(value) { - return serviceChartLabel(this.getLabelForValue(value as number), this.chart.width < SERVICE_CHART_COMPACT_PX); + return serviceLabel(this.getLabelForValue(value as number), this.chart.width < SERVICE_CHART_COMPACT_PX); }, }, }, diff --git a/frontend/src/lib/purchase-compatibility.ts b/frontend/src/lib/purchase-compatibility.ts index f630ba1e2..d9037f356 100644 --- a/frontend/src/lib/purchase-compatibility.ts +++ b/frontend/src/lib/purchase-compatibility.ts @@ -99,7 +99,7 @@ export const SAVINGS_PLANS_BUCKET_KEY = 'savings-plans'; // Pretty short label per SP plan type used inside the mixed-bucket // label (e.g. "Savings Plans (Compute + SageMaker)"). Mirrors the -// abbreviations in plans.ts:planServiceLabel without coupling to it. +// compact names in lib/service-labels.ts:serviceLabel without coupling to it. const SP_SHORT_LABEL: Record = { 'savings-plans-compute': 'Compute', 'savings-plans-ec2instance': 'EC2 Instance', diff --git a/frontend/src/lib/service-labels.ts b/frontend/src/lib/service-labels.ts new file mode 100644 index 000000000..42a27ee10 --- /dev/null +++ b/frontend/src/lib/service-labels.ts @@ -0,0 +1,34 @@ +// Display names for service slugs, shared by every page that shows a service +// (issues #1649, #1683). Slugs shared across providers (compute, cache, +// relational-db) get a provider-neutral name; the Provider column next to them +// carries the provider. Unknown slugs pass through unchanged. A Go test +// (internal/config/service_labels_frontend_test.go) checks every library +// ServiceType has an entry here. +const SERVICE_LABELS: Readonly> = { + ec2: 'EC2', + rds: 'RDS', + elasticache: 'ElastiCache', + opensearch: 'OpenSearch', + memorydb: 'MemoryDB', + redshift: 'Redshift', + 'savings-plans-compute': 'Compute Savings Plans', + 'savings-plans-ec2instance': 'EC2 Instance Savings Plans', + 'savings-plans-sagemaker': 'SageMaker Savings Plans', + 'savings-plans-database': 'Database Savings Plans', + savingsplans: 'Savings Plans', + compute: 'Compute', + 'relational-db': 'Relational DB', + cache: 'Cache', + search: 'Search', + nosql: 'NoSQL', + 'data-warehouse': 'Data Warehouse', + storage: 'Storage', + commitments: 'Commitments', + other: 'Other', +}; + +/** Display name for a service slug; `compact` abbreviates "Savings Plans" to "SP". */ +export function serviceLabel(slug: string, compact = false): string { + const label = SERVICE_LABELS[slug] ?? slug; + return compact ? label.replace(/ Savings Plans$/, ' SP') : label; +} diff --git a/frontend/src/plans.ts b/frontend/src/plans.ts index 6342de40e..4696ffb17 100644 --- a/frontend/src/plans.ts +++ b/frontend/src/plans.ts @@ -27,6 +27,7 @@ import { import { suggestedPurchaseStartDate } from './lib/calendar-date'; import { badgeHtml, statusBadgeHtml } from './lib/status-badge'; import { isPastScheduledDate } from './lib/purchase-row-guards'; +import { serviceLabel } from './lib/service-labels'; import { mountPlanEditorPanel } from './plan-editor-panel'; // pendingPlanRecommendations holds the resolved plan target captured at @@ -469,7 +470,7 @@ function renderPlannedPurchaseRow(purchase: PlannedPurchase): string { ${formatDate(purchase.scheduled_date)} ${providerBadgeHtml(purchase.provider)} - ${escapeHtml(purchase.service)} + ${escapeHtml(serviceLabel(purchase.service, true))} ${resourceCell} ${countCell} ${termCell} @@ -724,16 +725,6 @@ const LEGACY_PLAN_SERVICE_IDS: Readonly> = { 'gcp/memorystore': 'cache', }; -function planServiceLabel(slug: string): string { - switch (slug) { - case 'savings-plans-compute': return 'Compute SP'; - case 'savings-plans-ec2instance': return 'EC2 Instance SP'; - case 'savings-plans-sagemaker': return 'SageMaker SP'; - case 'savings-plans-database': return 'Database SP'; - default: return slug; - } -} - // Extract provider/service info from plan's services map. // // `service` is now a comma-separated list of all services covered by @@ -747,28 +738,31 @@ function planServiceLabel(slug: string): string { // are plan-level today, not per-service, so picking any entry is // correct. If the model ever differentiates per service, this needs // to render the same way. -function extractPlanInfo(plan: BackendPlan): { provider: string | null; service: string; term: number | null; coverage: number | null } { +// +// `service` holds the raw slugs (the edit flow selects the service option by +// it); `serviceDisplay` holds the display names for the card (#1683). +function extractPlanInfo(plan: BackendPlan): { provider: string | null; service: string; serviceDisplay: string; term: number | null; coverage: number | null } { const services = plan.services || {}; const serviceValues = Object.values(services); const firstService = serviceValues[0]; if (firstService) { - const service = serviceValues.length === 0 - ? '—' - : serviceValues - .map(s => planServiceLabel(s.service || '—')) - .join(', '); + const service = serviceValues.map(s => s.service || '—').join(', '); + const serviceDisplay = serviceValues + .map(s => (s.service ? serviceLabel(s.service, true) : '—')) + .join(', '); // H-4: never fabricate provider/term/coverage when absent. // Return null so callers can surface NO_VALUE/'Unknown' in the display // layer rather than silently committing to aws/3yr/80% defaults. return { provider: firstService.provider || null, service, + serviceDisplay, term: firstService.term || null, coverage: firstService.coverage ?? null }; } // No services at all: all fields are unknown. - return { provider: null, service: '—', term: null, coverage: null }; + return { provider: null, service: '—', serviceDisplay: '—', term: null, coverage: null }; } // Returns true when the plan's next_execution_date is strictly before today. @@ -866,7 +860,7 @@ function renderPlanCard(plan: BackendPlan, canManagePlan: boolean, canDeletePlan
Service - ${escapeHtml(info.service)} + ${escapeHtml(info.serviceDisplay)}
Term diff --git a/frontend/src/recommendations.ts b/frontend/src/recommendations.ts index 6452527f3..612fb0687 100644 --- a/frontend/src/recommendations.ts +++ b/frontend/src/recommendations.ts @@ -7,6 +7,7 @@ import * as state from './state'; import type { CostPeriod } from './state'; import { formatCurrency, formatTerm, escapeHtml, CURRENCY_DEFAULT_DIGITS, NO_VALUE } from './utils'; import { filterByEnabledProviders } from './lib/enabled-providers'; +import { serviceLabel } from './lib/service-labels'; import { getRecommendationsFreshness, refreshRecommendations as refreshRecommendationsAPI } from './api/recommendations'; import type { RecommendationsFreshness } from './api/recommendations'; import { showToast } from './toast'; @@ -895,7 +896,7 @@ async function onLookbackChange(rawValue: string): Promise { function savingsScopeText(visibleCount: number, loadedCount: number): string | null { const active = Object.keys(state.getRecommendationsColumnFilters()); if (loadedServerFilters.length > 0) { - const pushed = loadedServerFilters.map((f) => `${columnLabel(f.column)}: ${f.value}`); + const pushed = loadedServerFilters.map((f) => `${columnLabel(f.column)}: ${categoricalDisplayLabel(f.column, f.value)}`); const others = active.filter((col) => !loadedServerFilters.some((f) => f.column === col)).length; if (others > 0) pushed.push(`+${others} more ${others === 1 ? 'filter' : 'filters'}`); return `Filtered: ${visibleCount} recommendations (${pushed.join(', ')})`; @@ -2276,6 +2277,9 @@ function categoricalDisplayLabel( if (column === 'provider') { return providerDisplayName(value); } + if (column === 'service') { + return serviceLabel(value); + } return value; } @@ -3284,7 +3288,7 @@ function renderColumnCell(key: state.RecommendationsColumnId, rec: LocalRecommen case 'account': return `${escapeHtml(ctx.accountName)}`; case 'service': - return `${escapeHtml(rec.service)}`; + return `${escapeHtml(serviceLabel(rec.service))}`; case 'resource_type': return `${escapeHtml(rec.resource_type)}${rec.engine ? ` (${escapeHtml(rec.engine)})` : ''}${ctx.badge}`; case 'capacity': { @@ -3549,7 +3553,7 @@ function buildListMarkup( ${escapeHtml(providerDisplayName(childRep.provider))} ${escapeHtml(childAccountName)} - ${escapeHtml(childRep.service)} + ${escapeHtml(serviceLabel(childRep.service))} ${childIdentityParts.join(' — ')} ${childRangeParts.length > 0 ? `${childRangeParts.join(' · ')}` : ''} @@ -3633,7 +3637,7 @@ function buildListMarkup( ${chevronCell} ${escapeHtml(providerDisplayName(rep.provider))} ${escapeHtml(accountName)} - ${escapeHtml(rep.service)} + ${escapeHtml(serviceLabel(rep.service))} ${identityParts.join(' — ')} ${rangeParts.length > 0 ? `${rangeParts.join(' · ')}` : ''} diff --git a/frontend/tests-e2e/chart-text-alternative.spec.ts b/frontend/tests-e2e/chart-text-alternative.spec.ts index 3406dd40a..44d7cddfb 100644 --- a/frontend/tests-e2e/chart-text-alternative.spec.ts +++ b/frontend/tests-e2e/chart-text-alternative.spec.ts @@ -6,6 +6,12 @@ import { test, expect, type Page } from '@playwright/test'; import { mockApi, seedAuth, RECS } from './fixtures/recs'; +// Expected row headers, written out so a broken label helper can't pass its +// own check (#1649). Slugs without a display name pass through. +const SERVICE_NAMES: Record = { + aks: 'aks', compute: 'Compute', ec2: 'EC2', gce: 'gce', lambda: 'lambda', opensearch: 'OpenSearch', rds: 'RDS', +}; + async function openHome(page: Page): Promise { await seedAuth(page); await mockApi(page); @@ -47,8 +53,10 @@ test('the per-service chart has a value summary and a keyboard-operable table', await page.keyboard.press('Enter'); await expect(table).toBeVisible(); await expect(table.locator('tbody tr')).toHaveCount(services.size); + // #1649: the rows use the same display names as the chart axis. for (const svc of services) { - await expect(table.getByRole('rowheader', { name: svc, exact: true })).toBeVisible(); + expect(SERVICE_NAMES[svc], `add ${svc} to SERVICE_NAMES`).toBeDefined(); + await expect(table.getByRole('rowheader', { name: SERVICE_NAMES[svc]!, exact: true })).toBeVisible(); } }); diff --git a/frontend/tests-e2e/savings-kpi-parity.spec.ts b/frontend/tests-e2e/savings-kpi-parity.spec.ts index bade659aa..e568c43e8 100644 --- a/frontend/tests-e2e/savings-kpi-parity.spec.ts +++ b/frontend/tests-e2e/savings-kpi-parity.spec.ts @@ -89,5 +89,6 @@ test('the card stays labeled when the service filter is sent to the server', asy req.url().includes('/api/recommendations') && new URL(req.url()).searchParams.get('service') === 'ec2'); await page.locator('#opportunities-tab-btn').click(); await pushed; - await expect(scope).toHaveText(`Filtered: ${AWS_EC2_COUNT} recommendations (Service: ec2)`); + // #1683: the pushed-down filter is named like the Service column. + await expect(scope).toHaveText(`Filtered: ${AWS_EC2_COUNT} recommendations (Service: EC2)`); }); diff --git a/internal/config/service_labels_frontend_test.go b/internal/config/service_labels_frontend_test.go new file mode 100644 index 000000000..ff102a0df --- /dev/null +++ b/internal/config/service_labels_frontend_test.go @@ -0,0 +1,91 @@ +package config + +import ( + "go/ast" + "go/parser" + "go/token" + "os" + "os/exec" + "path/filepath" + "regexp" + "strconv" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +// TestEveryServiceTypeHasAFrontendLabel guards issues #1649 and #1683: every +// service slug the pinned library defines (every common.ServiceType constant) +// must have a display name in frontend/src/lib/service-labels.ts, so no page +// falls back to the raw slug. It reads the constants from the library source, +// so a constant added by a future bump fails here until it gets a label. +func TestEveryServiceTypeHasAFrontendLabel(t *testing.T) { + slugs := libraryServiceTypes(t) + require.Contains(t, slugs, "data-warehouse", "sanity check: the parser must find the library constants") + + src, err := os.ReadFile("../../frontend/src/lib/service-labels.ts") + require.NoError(t, err) + block := regexp.MustCompile(`(?s)const SERVICE_LABELS[^{]*\{(.*?)\n\};`).FindSubmatch(src) + require.NotNil(t, block, "SERVICE_LABELS map not found in service-labels.ts") + labeled := map[string]bool{} + for _, m := range regexp.MustCompile(`(?m)^\s*'?([a-z0-9-]+)'?\s*:`).FindAllSubmatch(block[1], -1) { + labeled[string(m[1])] = true + } + + for _, slug := range slugs { + require.Truef(t, labeled[slug], "service slug %q has no label in frontend/src/lib/service-labels.ts", slug) + } +} + +// libraryServiceTypes returns the values of every ServiceType constant in the +// pinned cloud-commitments-go common package. +func libraryServiceTypes(t *testing.T) []string { + t.Helper() + out, err := exec.Command("go", "list", "-m", "-f", "{{.Dir}}", "github.com/LeanerCloud/cloud-commitments-go/pkg").Output() + require.NoError(t, err, "locate the pinned cloud-commitments-go/pkg module") + dir := filepath.Join(strings.TrimSpace(string(out)), "common") + + files, err := filepath.Glob(filepath.Join(dir, "*.go")) + require.NoError(t, err) + + var slugs []string + fset := token.NewFileSet() + for _, path := range files { + if strings.HasSuffix(path, "_test.go") { + continue + } + file, err := parser.ParseFile(fset, path, nil, 0) + require.NoError(t, err) + slugs = append(slugs, serviceTypeConsts(t, file)...) + } + require.NotEmpty(t, slugs) + return slugs +} + +// serviceTypeConsts returns the string values of the ServiceType constants +// declared in file. +func serviceTypeConsts(t *testing.T, file *ast.File) []string { + t.Helper() + var slugs []string + for _, decl := range file.Decls { + gen, ok := decl.(*ast.GenDecl) + if !ok || gen.Tok != token.CONST { + continue + } + for _, spec := range gen.Specs { + vs := spec.(*ast.ValueSpec) + if id, ok := vs.Type.(*ast.Ident); !ok || id.Name != "ServiceType" { + continue + } + for _, v := range vs.Values { + if lit, ok := v.(*ast.BasicLit); ok && lit.Kind == token.STRING { + s, err := strconv.Unquote(lit.Value) + require.NoError(t, err) + slugs = append(slugs, s) + } + } + } + } + return slugs +}