From 2d31f996fb2cb84ba66faef2b0cc35561ff5d6ec Mon Sep 17 00:00:00 2001 From: Danilo Lakovic Date: Thu, 29 Oct 2026 18:35:58 +0100 Subject: [PATCH 1/4] Add regression tests for the URLs a repository is served at repositoryDistributions is tested against a mock server: the three kinds of binding, the paged scan across pages, a failed load rejecting with a readable message, and the not-served rule for deb. The file Distributions tab is tested for the server's base_url and the manifest line built from it. The list handler of the test utilities can now filter rows the way a server filter does. --- .../tab-distributions.test.tsx | 119 ++++++++ src/test-utils/pulp.test.ts | 12 + src/test-utils/pulp.ts | 19 +- .../repository-distributions.test.ts | 286 ++++++++++++++++++ 4 files changed, 432 insertions(+), 4 deletions(-) create mode 100644 src/containers/file-repository/tab-distributions.test.tsx create mode 100644 src/utilities/repository-distributions.test.ts diff --git a/src/containers/file-repository/tab-distributions.test.tsx b/src/containers/file-repository/tab-distributions.test.tsx new file mode 100644 index 00000000..413bff09 --- /dev/null +++ b/src/containers/file-repository/tab-distributions.test.tsx @@ -0,0 +1,119 @@ +import { screen, within } from '@testing-library/react'; +import { describe, expect, it, vi } from 'vitest'; +import { distribution, repository } from 'src/test-utils/pulp'; +import { renderWithProviders } from 'src/test-utils/render'; +import { type RepositoryDistribution } from 'src/utilities/repository-distributions'; +import { DistributionsTab } from './tab-distributions'; + +// A distribution as the repository page passes it to the tab: the page has +// already loaded and merged them. +function loaded( + overrides: Partial = {}, +): RepositoryDistribution { + const base = distribution(); + return { + pulp_href: base.pulp_href, + name: base.name, + base_path: base.base_path, + url: base.base_url, + served: true, + binding: 'repository', + content_guard: null, + pulp_created: base.pulp_created, + ...overrides, + }; +} + +function renderTab( + distributions: RepositoryDistribution[], + distributionsError: string | null = null, +) { + const addAlert = vi.fn(); + const item = { + ...repository({ name: 'tools' }), + distributions, + distributionsError, + }; + renderWithProviders( + true, + }} + />, + ); + return { addAlert }; +} + +describe('file repository Distributions tab', () => { + it('shows the base_url the server reports and a CLI line that ends with that URL plus PULP_MANIFEST', async () => { + const url = 'https://pulp.example/pulp/content/served/tools/'; + renderTab([loaded({ name: 'tools-dist', base_path: 'tools', url })]); + + const row = (await screen.findByText('tools-dist')).closest('tr'); + expect(row).not.toBeNull(); + const cells = within(row as HTMLElement); + + // the URL cell and the CLI line both carry the server's URL + const occurrences = cells.getAllByText((content) => content.includes(url), { + selector: 'span, input, code, p', + }); + expect(occurrences.length).toBeGreaterThanOrEqual(2); + + const cliLine = cells.getByText(/PULP_MANIFEST/); + expect(cliLine.textContent).toContain(`${url}PULP_MANIFEST`); + }); + + it('shows no manifest line for a distribution the server gave no URL', async () => { + renderTab([loaded({ name: 'no-url', url: null })]); + + const row = (await screen.findByText('no-url')).closest('tr'); + + expect(within(row as HTMLElement).queryByText(/PULP_MANIFEST/)).toBeNull(); + }); + + it('lists every distribution it was given with its own URL', async () => { + renderTab([ + loaded({ name: 'first', url: 'https://pulp.example/pulp/content/a/' }), + loaded({ + name: 'second', + url: 'https://pulp.example/pulp/content/b/', + binding: 'publication', + }), + ]); + + const first = (await screen.findByText('first')).closest('tr'); + const second = screen.getByText('second').closest('tr'); + + expect(first?.textContent).toContain( + 'https://pulp.example/pulp/content/a/PULP_MANIFEST', + ); + expect(second?.textContent).toContain( + 'https://pulp.example/pulp/content/b/PULP_MANIFEST', + ); + }); + + it('shows the not served label next to a distribution without a publication', async () => { + renderTab([loaded({ name: 'bare', served: false })]); + + const row = (await screen.findByText('bare')).closest('tr'); + + expect( + within(row as HTMLElement).getByText(/Not served: no publication/), + ).toBeInTheDocument(); + }); + + it('reports a failed distribution load in an alert instead of listing nothing silently', async () => { + const { addAlert } = renderTab([], 'Error 500 - the server failed'); + + await screen.findByText(/No distribution serves this repository/); + + expect(addAlert).toHaveBeenCalledTimes(1); + expect(addAlert.mock.calls[0][0]).toMatchObject({ + variant: 'danger', + description: 'Error 500 - the server failed', + }); + }); +}); diff --git a/src/test-utils/pulp.test.ts b/src/test-utils/pulp.test.ts index d14af255..3022cd11 100644 --- a/src/test-utils/pulp.test.ts +++ b/src/test-utils/pulp.test.ts @@ -43,6 +43,18 @@ describe('listOf', () => { expect(body.count).toBe(5); }); + it('answers only the rows the matching filter selects, with their count', async () => { + const { handler } = listOf(REPOSITORIES, rows, { + matching: (row, query) => row === query.get('name'), + }); + server.use(handler); + + const body = await read('name=c'); + + expect(body.results).toStrictEqual(['c']); + expect(body.count).toBe(1); + }); + it('links to the next page until the last page, which has none', async () => { const { handler } = listOf(REPOSITORIES, rows, { pageSize: 2 }); server.use(handler); diff --git a/src/test-utils/pulp.ts b/src/test-utils/pulp.ts index 41027493..5b3f718c 100644 --- a/src/test-utils/pulp.ts +++ b/src/test-utils/pulp.ts @@ -31,18 +31,29 @@ const DEFAULT_PAGE_SIZE = 100; // A GET handler for a list endpoint, to pass to server.use. It answers offset // and limit requests with slices of the rows and a next link until the last // page, as the server does; pageSize caps the rows of one page whatever the -// request asks for. The query of every request it answers is recorded in -// queries, in order, for assertions. +// request asks for. matching keeps only the rows a filter query selects, the +// way a server filter does; without it every row is answered whatever the +// query says. The query of every request it answers is recorded in queries, in +// order, for assertions. export function listOf( path: string, - rows: T[], - { pageSize = DEFAULT_PAGE_SIZE }: { pageSize?: number } = {}, + allRows: T[], + { + pageSize = DEFAULT_PAGE_SIZE, + matching, + }: { + pageSize?: number; + matching?: (row: T, query: URLSearchParams) => boolean; + } = {}, ) { const queries: URLSearchParams[] = []; const handler = http.get(`*${path}`, ({ request }) => { const url = new URL(request.url); queries.push(new URLSearchParams(url.searchParams)); + const rows = matching + ? allRows.filter((row) => matching(row, url.searchParams)) + : allRows; const offset = Math.max(0, Number(url.searchParams.get('offset')) || 0); const requested = Number(url.searchParams.get('limit')) || pageSize; diff --git a/src/utilities/repository-distributions.test.ts b/src/utilities/repository-distributions.test.ts new file mode 100644 index 00000000..bc980b0e --- /dev/null +++ b/src/utilities/repository-distributions.test.ts @@ -0,0 +1,286 @@ +import { HttpResponse, http } from 'msw'; +import { describe, expect, it } from 'vitest'; +import { + type IDistribution, + type IPublication, + distribution, + listOf, + publication, + repository, +} from 'src/test-utils/pulp'; +import { server } from 'src/test-utils/server'; +import { + distributionsLoadError, + repositoryDistributions, +} from './repository-distributions'; + +const FILE_DISTRIBUTIONS = '/pulp/api/v3/distributions/file/file/'; +const FILE_PUBLICATIONS = '/pulp/api/v3/publications/file/file/'; +const DEB_DISTRIBUTIONS = '/pulp/api/v3/distributions/deb/apt/'; +const DEB_APT_PUBLICATIONS = '/pulp/api/v3/publications/deb/apt/'; +const DEB_VERBATIM_PUBLICATIONS = '/pulp/api/v3/publications/deb/verbatim/'; + +// the server's repository filter on a list endpoint +const ofRepository = (row: { repository: string | null }, q: URLSearchParams) => + row.repository === q.get('repository'); + +// Serves the given rows on the file plugin's distribution and publication +// endpoints, filtering by repository where the server does, and returns the +// recorded queries of both. +function serveFile( + distributions: IDistribution[], + publications: IPublication[], +) { + const distributionList = listOf(FILE_DISTRIBUTIONS, distributions, { + pageSize: 1000, + matching: (row, query) => + query.has('repository') ? ofRepository(row, query) : true, + }); + const publicationList = listOf(FILE_PUBLICATIONS, publications, { + pageSize: 1000, + matching: ofRepository, + }); + server.use(distributionList.handler, publicationList.handler); + return { + distributionQueries: distributionList.queries, + publicationQueries: publicationList.queries, + }; +} + +describe('repositoryDistributions', () => { + it('finds the distributions bound to the repository, to one of its publications and to one of its versions, each with the server base_url', async () => { + const repo = repository(); + const other = repository(); + const repoPublication = publication({ + repository: repo.pulp_href, + repository_version: `${repo.pulp_href}versions/1/`, + }); + const otherPublication = publication({ repository: other.pulp_href }); + + const boundToRepository = distribution({ + base_path: 'by-repository', + base_url: 'https://pulp.example/content/served-by-repository/', + repository: repo.pulp_href, + }); + const boundToPublication = distribution({ + base_path: 'by-publication', + base_url: 'https://pulp.example/content/served-by-publication/', + publication: repoPublication.pulp_href, + }); + const boundToVersion = distribution({ + base_path: 'by-version', + base_url: 'https://pulp.example/content/served-by-version/', + repository_version: `${repo.pulp_href}versions/1/`, + }); + const unrelated = [ + distribution({ + base_path: 'other-repository', + repository: other.pulp_href, + }), + distribution({ + base_path: 'other-publication', + publication: otherPublication.pulp_href, + }), + distribution({ + base_path: 'other-version', + repository_version: `${other.pulp_href}versions/0/`, + }), + distribution({ base_path: 'bound-to-nothing' }), + ]; + + serveFile( + [boundToRepository, boundToPublication, boundToVersion, ...unrelated], + [repoPublication, otherPublication], + ); + + const found = await repositoryDistributions('file', repo.pulp_href); + + expect( + found.map(({ name, binding, url }) => ({ name, binding, url })), + ).toStrictEqual([ + { + name: 'by-publication', + binding: 'publication', + url: 'https://pulp.example/content/served-by-publication/', + }, + { + name: 'by-repository', + binding: 'repository', + url: 'https://pulp.example/content/served-by-repository/', + }, + { + name: 'by-version', + binding: 'repository_version', + url: 'https://pulp.example/content/served-by-version/', + }, + ]); + expect(found.every(({ served }) => served)).toBe(true); + }); + + it('asks the server for the repository filter, the url field and the repository publications', async () => { + const repo = repository(); + const { distributionQueries, publicationQueries } = serveFile([], []); + + await repositoryDistributions('file', repo.pulp_href); + + const filtered = distributionQueries.filter((q) => q.has('repository')); + const scans = distributionQueries.filter((q) => !q.has('repository')); + + expect(filtered).toHaveLength(1); + expect(filtered[0].get('repository')).toBe(repo.pulp_href); + expect(filtered[0].get('fields')).toContain('base_url'); + expect(scans).toHaveLength(1); + expect(scans[0].get('fields')).toContain('base_url'); + expect(publicationQueries).toHaveLength(1); + expect(publicationQueries[0].get('repository')).toBe(repo.pulp_href); + }); + + it('follows the paged scan to later pages for a distribution bound to a version', async () => { + const repo = repository(); + const filler = Array.from({ length: 599 }, (_, index) => + distribution({ base_path: `filler-${index}` }), + ); + const late = distribution({ + base_path: 'late-version', + repository_version: `${repo.pulp_href}versions/3/`, + }); + // the matching one sits on the second page of 500 + const everything = [...filler.slice(0, 550), late, ...filler.slice(550)]; + + const { distributionQueries } = serveFile(everything, []); + + const found = await repositoryDistributions('file', repo.pulp_href); + + expect(found.map(({ name }) => name)).toStrictEqual(['late-version']); + const scanOffsets = distributionQueries + .filter((q) => !q.has('repository')) + .map((q) => Number(q.get('offset'))); + expect(scanOffsets).toStrictEqual([0, 500]); + }); + + it('rejects with a readable message when the distribution list answers 500, never an empty list', async () => { + const repo = repository(); + server.use( + http.get(`*${FILE_DISTRIBUTIONS}`, () => + HttpResponse.json({ detail: 'boom' }, { status: 500 }), + ), + listOf(FILE_PUBLICATIONS, []).handler, + ); + + const failure = await repositoryDistributions('file', repo.pulp_href).then( + (list) => ({ list }), + (error) => ({ error }), + ); + + expect(failure).not.toHaveProperty('list'); + expect( + distributionsLoadError((failure as { error: unknown }).error), + ).toMatch(/^Error 500 - .*server encountered an error/); + }); + + it('rejects when only the publication list fails, so a partial answer is not shown as complete', async () => { + const repo = repository(); + server.use( + listOf(FILE_DISTRIBUTIONS, [distribution({ repository: repo.pulp_href })]) + .handler, + http.get(`*${FILE_PUBLICATIONS}`, () => + HttpResponse.json({ detail: 'nope' }, { status: 403 }), + ), + ); + + const failure = await repositoryDistributions('file', repo.pulp_href).then( + () => null, + (error: unknown) => error, + ); + + expect(failure).not.toBeNull(); + expect(distributionsLoadError(failure)).toMatch(/^Error 403 - /); + }); + + it('describes a failure without a response by its own message', () => { + expect(distributionsLoadError(new Error('Network Error'))).toBe( + 'Network Error', + ); + }); + + describe('deb', () => { + function serveDeb( + distributions: IDistribution[], + aptPublications: IPublication[], + verbatimPublications: IPublication[], + ) { + server.use( + listOf(DEB_DISTRIBUTIONS, distributions, { + pageSize: 1000, + matching: (row, query) => + query.has('repository') ? ofRepository(row, query) : true, + }).handler, + listOf(DEB_APT_PUBLICATIONS, aptPublications, { + pageSize: 1000, + matching: ofRepository, + }).handler, + listOf(DEB_VERBATIM_PUBLICATIONS, verbatimPublications, { + pageSize: 1000, + matching: ofRepository, + }).handler, + ); + } + + it('marks a repository-bound distribution not served while the repository has no publication', async () => { + const repo = repository(); + serveDeb( + [ + distribution({ + base_path: 'unpublished', + repository: repo.pulp_href, + }), + ], + [], + [], + ); + + const found = await repositoryDistributions('deb', repo.pulp_href); + + expect(found).toHaveLength(1); + expect(found[0].served).toBe(false); + }); + + it('marks it served once the repository has a verbatim publication', async () => { + const repo = repository(); + serveDeb( + [distribution({ base_path: 'published', repository: repo.pulp_href })], + [], + [publication({ repository: repo.pulp_href })], + ); + + const found = await repositoryDistributions('deb', repo.pulp_href); + + expect(found).toHaveLength(1); + expect(found[0].served).toBe(true); + }); + + it('marks a version-bound distribution not served when only another version is published', async () => { + const repo = repository(); + serveDeb( + [ + distribution({ + base_path: 'pinned', + repository_version: `${repo.pulp_href}versions/2/`, + }), + ], + [ + publication({ + repository: repo.pulp_href, + repository_version: `${repo.pulp_href}versions/1/`, + }), + ], + [], + ); + + const found = await repositoryDistributions('deb', repo.pulp_href); + + expect(found).toHaveLength(1); + expect(found[0].served).toBe(false); + }); + }); +}); From 8d7b04e10e4732fd6e66b3c31aef97d347defceb Mon Sep 17 00:00:00 2001 From: Danilo Lakovic Date: Thu, 29 Oct 2026 18:38:00 +0100 Subject: [PATCH 2/4] Add regression tests for the repository create flow and the build stamp check The shared create flow is tested by the requests it sends: blocked before the first POST when the name or base path is taken or cannot be checked, the repository then the distribution in that order, and a refused distribution leaving the repository alone with the server's reason and no retry or rollback. The settings module is tested to refuse a malformed build commit with a message naming PULP_UI_COMMIT. The node test project now also collects tests under config/. --- config/shared.config.test.ts | 65 +++++ src/utilities/create-repository.test.ts | 330 ++++++++++++++++++++++++ vitest.config.ts | 5 +- 3 files changed, 398 insertions(+), 2 deletions(-) create mode 100644 config/shared.config.test.ts create mode 100644 src/utilities/create-repository.test.ts diff --git a/config/shared.config.test.ts b/config/shared.config.test.ts new file mode 100644 index 00000000..f70108a8 --- /dev/null +++ b/config/shared.config.test.ts @@ -0,0 +1,65 @@ +// @vitest-environment node +// The webpack settings module checks the commit the build is stamped with as +// soon as it is loaded. A short hash or a branch name would not pin a build to +// one source tree, so loading must stop with a message that names the +// variable, before any bundling starts. +import { createRequire } from 'node:module'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; + +const require = createRequire(import.meta.url); +const CONFIG = require.resolve('./shared.config.js'); + +const VARIABLES = ['PULP_UI_COMMIT', 'PULP_UI_CONFIG', 'PULP_UI_DATE']; +let saved: Record; + +beforeEach(() => { + saved = Object.fromEntries( + VARIABLES.map((name) => [name, process.env[name]]), + ); + VARIABLES.forEach((name) => delete process.env[name]); + delete require.cache[CONFIG]; +}); + +afterEach(() => { + for (const name of VARIABLES) { + if (saved[name] === undefined) { + delete process.env[name]; + } else { + process.env[name] = saved[name]; + } + } + delete require.cache[CONFIG]; +}); + +// Loads the module afresh with the given commit in the environment. +function load(commit: string) { + process.env['PULP_UI_COMMIT'] = commit; + delete require.cache[CONFIG]; + return require(CONFIG); +} + +describe('the build stamp commit', () => { + it.each([ + ['a short hash', 'abc1234'], + ['a branch name', 'main'], + ['a hash with upper case letters', 'A'.repeat(40)], + ['a hash one character short', 'a'.repeat(39)], + ['a hash one character long', 'a'.repeat(41)], + ['a hash with surrounding text', ` ${'a'.repeat(40)}`], + ])( + 'is refused when it is %s, with a message naming PULP_UI_COMMIT', + (_, commit) => { + expect(() => load(commit)).toThrowError(/PULP_UI_COMMIT/); + }, + ); + + it('quotes the refused value so the mistake can be found', () => { + expect(() => load('abc1234')).toThrowError('"abc1234"'); + }); + + it('is accepted when it is a full lowercase 40-character hash', () => { + const config = load('0123456789abcdef'.repeat(2) + '01234567'); + + expect(typeof config).toBe('function'); + }); +}); diff --git a/src/utilities/create-repository.test.ts b/src/utilities/create-repository.test.ts new file mode 100644 index 00000000..af3ebbeb --- /dev/null +++ b/src/utilities/create-repository.test.ts @@ -0,0 +1,330 @@ +import { HttpResponse, http } from 'msw'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { FileDistributionAPI } from 'src/api/file-distribution'; +import { FileRepositoryAPI } from 'src/api/file-repository'; +import { type IDistribution, distribution, listOf } from 'src/test-utils/pulp'; +import { server } from 'src/test-utils/server'; +import { CreateBlocked, createRepository } from './create-repository'; + +const DISTRIBUTIONS = '/pulp/api/v3/distributions/'; +const REPOSITORY_HREF = + '/pulp/api/v3/repositories/file/file/00000000-0000-4000-8000-0000000000aa/'; +const TASK_HREF = '/pulp/api/v3/tasks/00000000-0000-4000-8000-0000000000bb/'; + +const NAME = 'Acme Tools'; +const BASE_PATH = 'acme-tools'; + +interface Seen { + method: string; + path: string; +} + +// every request the code under test makes, in order, and the JSON body of +// every POST +let seen: Seen[]; +let postBodies: Record[]; + +beforeEach(() => { + seen = []; + postBodies = []; + server.events.on('request:start', ({ request }) => { + seen.push({ method: request.method, path: new URL(request.url).pathname }); + }); +}); + +afterEach(() => { + server.events.removeAllListeners(); +}); + +const jsonBody = async (request: Request) => + (await request.json()) as Record; + +const requestsOf = (method: string, pathPart: string) => + seen.filter((r) => r.method === method && r.path.includes(pathPart)); + +const posts = () => seen.filter((r) => r.method === 'POST'); + +// The generic distribution list the availability check reads, filtering by +// the three queries the check sends: exact name, a set of base paths and a +// base path fragment. +function serveExistingDistributions(existing: IDistribution[]) { + const { handler, queries } = listOf(DISTRIBUTIONS, existing, { + matching: (row, query) => { + if (query.has('name')) { + return row.name === query.get('name'); + } + if (query.has('base_path__in')) { + return query.get('base_path__in').split(',').includes(row.base_path); + } + if (query.has('base_path__contains')) { + return row.base_path.includes(query.get('base_path__contains')); + } + return true; + }, + }); + server.use(handler); + return queries; +} + +function serveRepositoryCreate() { + server.use( + http.post('*/pulp/api/v3/repositories/file/file/', async ({ request }) => { + postBodies.push(await jsonBody(request)); + return HttpResponse.json({ pulp_href: REPOSITORY_HREF }, { status: 201 }); + }), + ); +} + +function serveRepositoryRefusal() { + server.use( + http.post('*/pulp/api/v3/repositories/file/file/', async ({ request }) => { + postBodies.push(await jsonBody(request)); + return HttpResponse.json( + { name: ['This field must be unique.'] }, + { status: 400 }, + ); + }), + ); +} + +function serveDistributionCreate(answer: () => Response | Promise) { + server.use( + http.post('*/pulp/api/v3/distributions/file/file/', async ({ request }) => { + postBodies.push(await jsonBody(request)); + return answer(); + }), + ); +} + +function serveTask(state: string, error?: { description: string }) { + server.use( + http.get('*/pulp/api/v3/tasks/:id/', () => + HttpResponse.json({ pulp_href: TASK_HREF, state, error }), + ), + ); +} + +const create = (distributionRequest: { basePath: string } | null) => + createRepository({ + RepositoryAPI: FileRepositoryAPI, + DistributionAPI: FileDistributionAPI, + data: { name: NAME }, + distribution: distributionRequest, + }); + +describe('createRepository', () => { + describe('when the base path or the name is taken', () => { + it('is blocked with no POST when another plugin distribution already has the base path', async () => { + serveExistingDistributions([ + distribution({ + name: 'someone-else', + base_path: BASE_PATH, + pulp_href: '/pulp/api/v3/distributions/rpm/rpm/1/', + }), + ]); + + const outcome = await create({ basePath: BASE_PATH }).then( + () => null, + (e: unknown) => e, + ); + + expect(outcome).toBeInstanceOf(CreateBlocked); + expect((outcome as CreateBlocked).errors['base_path']).toContain( + 'someone-else', + ); + expect(posts()).toStrictEqual([]); + }); + + it('is blocked with no POST when a distribution sits below the base path', async () => { + serveExistingDistributions([ + distribution({ name: 'nested', base_path: `${BASE_PATH}/sub` }), + ]); + + const outcome = await create({ basePath: BASE_PATH }).catch( + (e: unknown) => e, + ); + + expect(outcome).toBeInstanceOf(CreateBlocked); + expect(posts()).toStrictEqual([]); + }); + + it('is blocked with no POST when a distribution sits above the base path', async () => { + serveExistingDistributions([ + distribution({ name: 'parent', base_path: 'acme' }), + ]); + + const outcome = await create({ basePath: 'acme/tools' }).catch( + (e: unknown) => e, + ); + + expect(outcome).toBeInstanceOf(CreateBlocked); + expect(posts()).toStrictEqual([]); + }); + + it('is blocked with no POST when a distribution already has the name', async () => { + serveExistingDistributions([ + distribution({ name: NAME, base_path: 'something-else' }), + ]); + + const outcome = await create({ basePath: BASE_PATH }).catch( + (e: unknown) => e, + ); + + expect(outcome).toBeInstanceOf(CreateBlocked); + expect((outcome as CreateBlocked).errors['name']).toContain(NAME); + expect(posts()).toStrictEqual([]); + }); + + it('is blocked with no POST when the check itself cannot run', async () => { + server.use( + http.get(`*${DISTRIBUTIONS}`, () => + HttpResponse.json({ detail: 'broken' }, { status: 500 }), + ), + ); + + const outcome = await create({ basePath: BASE_PATH }).catch( + (e: unknown) => e, + ); + + expect(outcome).toBeInstanceOf(CreateBlocked); + expect((outcome as CreateBlocked).message).toContain( + 'Nothing was created', + ); + expect(posts()).toStrictEqual([]); + }); + + it('compares the name and path the caller gave, without altering them', async () => { + const queries = serveExistingDistributions([]); + serveRepositoryCreate(); + serveDistributionCreate(() => + HttpResponse.json({ task: TASK_HREF }, { status: 202 }), + ); + serveTask('completed'); + + await create({ basePath: 'acme/tools' }); + + expect(queries.map((q) => q.get('name')).filter(Boolean)).toStrictEqual([ + NAME, + ]); + expect( + queries.map((q) => q.get('base_path__in')).filter(Boolean), + ).toStrictEqual(['acme,acme/tools']); + }); + }); + + describe('when both are free', () => { + it('checks first, then POSTs the repository, then the distribution with the chosen base path and the new repository', async () => { + serveExistingDistributions([]); + serveRepositoryCreate(); + serveDistributionCreate(() => + HttpResponse.json({ task: TASK_HREF }, { status: 202 }), + ); + serveTask('completed'); + + const result = await create({ basePath: BASE_PATH }); + + expect(result).toStrictEqual({ + repositoryHref: REPOSITORY_HREF, + distributionError: null, + }); + expect(posts().map((r) => r.path)).toStrictEqual([ + '/pulp/api/v3/repositories/file/file/', + '/pulp/api/v3/distributions/file/file/', + ]); + expect(postBodies).toStrictEqual([ + { name: NAME }, + { name: NAME, base_path: BASE_PATH, repository: REPOSITORY_HREF }, + ]); + + // every availability lookup happened before the first write + const firstPost = seen.findIndex((r) => r.method === 'POST'); + const lastLookup = seen.map((r) => r.path).lastIndexOf(DISTRIBUTIONS); + expect(lastLookup).toBeGreaterThanOrEqual(0); + expect(lastLookup).toBeLessThan(firstPost); + }); + + it('creates only the repository, with no distribution request, when none was asked for', async () => { + serveRepositoryCreate(); + + const result = await create(null); + + expect(result).toStrictEqual({ + repositoryHref: REPOSITORY_HREF, + distributionError: null, + }); + expect(posts().map((r) => r.path)).toStrictEqual([ + '/pulp/api/v3/repositories/file/file/', + ]); + expect(postBodies).toStrictEqual([{ name: NAME }]); + }); + }); + + describe('when something fails after the checks passed', () => { + it('keeps the repository, makes no second distribution POST and no DELETE, and reports the server reason when the distribution is refused', async () => { + serveExistingDistributions([]); + serveRepositoryCreate(); + serveDistributionCreate(() => + HttpResponse.json( + { detail: 'You do not have permission to perform this action.' }, + { status: 403 }, + ), + ); + + const result = await create({ basePath: BASE_PATH }); + + expect(result.repositoryHref).toBe(REPOSITORY_HREF); + expect(result.distributionError).toContain( + 'You do not have permission to perform this action.', + ); + expect(requestsOf('POST', '/distributions/file/file/')).toHaveLength(1); + expect(requestsOf('POST', '/repositories/file/file/')).toHaveLength(1); + expect(seen.filter((r) => r.method === 'DELETE')).toStrictEqual([]); + }); + + it('reports the task error and never retries when the distribution task fails', async () => { + serveExistingDistributions([]); + serveRepositoryCreate(); + serveDistributionCreate(() => + HttpResponse.json({ task: TASK_HREF }, { status: 202 }), + ); + serveTask('failed', { description: 'base path overlaps another one' }); + + const result = await create({ basePath: BASE_PATH }); + + expect(result.distributionError).toBe('base path overlaps another one'); + expect(requestsOf('POST', '/distributions/file/file/')).toHaveLength(1); + expect(seen.filter((r) => r.method === 'DELETE')).toStrictEqual([]); + }); + + it('rejects with the server error and sends no distribution request when the repository is refused', async () => { + serveExistingDistributions([]); + serveRepositoryRefusal(); + + const outcome = await create({ basePath: BASE_PATH }).catch( + (e: unknown) => e, + ); + + expect(outcome).not.toBeInstanceOf(CreateBlocked); + expect( + (outcome as { response: { status: number } }).response.status, + ).toBe(400); + expect(requestsOf('POST', '/distributions/')).toStrictEqual([]); + expect(requestsOf('POST', '/repositories/file/file/')).toHaveLength(1); + }); + }); + + it('never sends a name that differs from the one given, whatever happens', async () => { + serveExistingDistributions([]); + serveRepositoryCreate(); + serveDistributionCreate(() => + HttpResponse.json({ detail: 'refused' }, { status: 403 }), + ); + + await create({ basePath: BASE_PATH }); + + expect(postBodies.length).toBeGreaterThan(0); + for (const body of postBodies) { + expect(body['name']).toBe(NAME); + } + }); +}); diff --git a/vitest.config.ts b/vitest.config.ts index 5bc19dfa..825e1106 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -5,7 +5,8 @@ import { defineConfig } from 'vitest/config'; // Two projects keep the browser-like setup away from Node tooling tests: // "app" runs the application tests under src/ in a DOM environment with the // HTTP mock server and the lingui macro transform, "node" runs the tests of the -// repository scripts under scripts/ with no setup at all. The sdk workspace +// repository scripts under scripts/ and of the build configuration under +// config/ with no setup at all. The sdk workspace // keeps its own configuration and is run through its own test script, so // nothing under packages/ is collected here. There is deliberately no // passWithNoTests: an include pattern that matches no file must fail the run, @@ -52,7 +53,7 @@ export default defineConfig({ test: { name: 'node', environment: 'node', - include: ['scripts/**/*.test.{ts,mjs}'], + include: ['scripts/**/*.test.{ts,mjs}', 'config/**/*.test.ts'], exclude, }, }, From 25a7147c8c16968a70ffdb2dbeffdc03ebf64bcc Mon Sep 17 00:00:00 2001 From: Danilo Lakovic Date: Thu, 29 Oct 2026 18:42:19 +0100 Subject: [PATCH 3/4] Add regression tests for the repository form The create-distribution box is offered unticked on create only, a base path under a configured reserved prefix is refused with Save blocked, retained versions starts empty and shows the server's refusal, and a double click on Save reaches the save handler once, including a second click that arrives before the page has re-rendered. --- src/components/repository-form.test.tsx | 298 ++++++++++++++++++++++++ 1 file changed, 298 insertions(+) create mode 100644 src/components/repository-form.test.tsx diff --git a/src/components/repository-form.test.tsx b/src/components/repository-form.test.tsx new file mode 100644 index 00000000..29000377 --- /dev/null +++ b/src/components/repository-form.test.tsx @@ -0,0 +1,298 @@ +import { act, screen, waitFor } from '@testing-library/react'; +import { HttpResponse, http } from 'msw'; +import { useState } from 'react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { listOf } from 'src/test-utils/pulp'; +import { renderWithProviders } from 'src/test-utils/render'; +import { server } from 'src/test-utils/server'; +import { RepositoryForm } from './repository-form'; + +// The configuration is loaded once when the application starts, so a test that +// needs a different one replaces the module. The reserved prefixes are a +// made-up list, not those of any deployment. +const settings = vi.hoisted(() => ({ reservedPrefixes: [] as string[] })); + +vi.mock('src/ui-config', () => ({ + configPromise: Promise.resolve(), + configFallback: () => undefined, + get config() { + return { + API_BASE_PATH: '/pulp/api/v3/', + UI_BASE_PATH: '/ui/', + RESERVED_BASE_PATH_PREFIXES: settings.reservedPrefixes, + }; + }, +})); + +const API = '/pulp/api/v3'; + +beforeEach(() => { + settings.reservedPrefixes = []; + server.use( + listOf(`${API}/remotes/file/file/`, []).handler, + listOf(`${API}/distributions/file/file/`, []).handler, + listOf(`${API}/publications/file/file/`, []).handler, + http.get(`*${API}/status/`, () => + HttpResponse.json({ + content_settings: { + content_origin: 'https://pulp.example', + content_path_prefix: '/pulp/content/', + }, + }), + ), + ); +}); + +type Save = (options: { distribution: { basePath: string } | null }) => unknown; + +// The form is controlled, so a small parent keeps the repository it edits the +// way the edit pages do. +function Harness({ + isNew, + onSave, + errorMessages = {}, + initial = {}, +}: { + isNew: boolean; + onSave: Save; + errorMessages?: Record; + initial?: Record; +}) { + const [repository, setRepository] = useState({ + name: isNew ? '' : 'existing', + description: '', + retain_repo_versions: null, + pulp_labels: {}, + remote: null, + pulp_href: isNew + ? undefined + : `${API}/repositories/file/file/00000000-0000-4000-8000-000000000001/`, + ...initial, + }); + + return ( + undefined} + onSave={onSave as never} + plugin='file' + repository={repository as never} + updateRepository={setRepository as never} + /> + ); +} + +const distributionBox = () => + screen.queryByRole('checkbox', { name: /Create a distribution/ }); +const saveButton = () => screen.getByRole('button', { name: 'Save' }); +const basePathInput = () => screen.getByRole('textbox', { name: /Base path/ }); +const retainInput = () => screen.getByRole('spinbutton', { name: /Retained/ }); + +describe('repository form, distribution option', () => { + it('offers the create-distribution box unticked on create, with no base path field yet', async () => { + renderWithProviders(); + + const box = await screen.findByRole('checkbox', { + name: /Create a distribution/, + }); + + expect(box).not.toBeChecked(); + expect(screen.queryByRole('textbox', { name: /Base path/ })).toBeNull(); + }); + + it('shows the base path and the warning about a missing content guard once the box is ticked', async () => { + const { user } = renderWithProviders(); + + await user.click( + await screen.findByRole('checkbox', { name: /Create a distribution/ }), + ); + + expect(basePathInput()).toBeInTheDocument(); + expect(screen.getByText(/no content guard/)).toBeInTheDocument(); + }); + + it('follows the repository name for the base path until the user types one, and previews the server URL', async () => { + const { user } = renderWithProviders(); + + await user.type( + await screen.findByRole('textbox', { name: /Name/ }), + 'tools', + ); + await user.click(distributionBox()); + + expect(basePathInput()).toHaveValue('tools'); + expect( + await screen.findByText( + 'URL (preview): https://pulp.example/pulp/content/tools/', + ), + ).toBeInTheDocument(); + }); + + it('does not render the box or a base path field when editing, and Save asks for no distribution', async () => { + const onSave = vi.fn(); + const { user } = renderWithProviders( + , + ); + + await screen.findByRole('button', { name: 'Save' }); + await user.click(saveButton()); + + expect(distributionBox()).toBeNull(); + expect(screen.queryByRole('textbox', { name: /Base path/ })).toBeNull(); + expect(onSave).toHaveBeenCalledTimes(1); + expect(onSave).toHaveBeenCalledWith({ distribution: null }); + }); + + it('asks for a distribution at the typed base path only when the box is ticked', async () => { + const onSave = vi.fn(); + const { user } = renderWithProviders(); + + await user.click(await screen.findByRole('button', { name: 'Save' })); + expect(onSave).toHaveBeenLastCalledWith({ distribution: null }); + + await user.click(distributionBox()); + await user.type(basePathInput(), 'tools/stable'); + await user.click(saveButton()); + + expect(onSave).toHaveBeenLastCalledWith({ + distribution: { basePath: 'tools/stable' }, + }); + }); +}); + +describe('repository form, retained versions', () => { + it('leaves the field empty for a repository that keeps every version', async () => { + renderWithProviders(); + + expect( + await screen.findByRole('spinbutton', { name: /Retained/ }), + ).toHaveValue(null); + }); + + it('shows the server field error under the field when the value was refused', async () => { + renderWithProviders( + , + ); + + expect( + await screen.findByText(/greater than or equal to 1/), + ).toBeInTheDocument(); + expect(retainInput()).toHaveValue(0); + expect(retainInput()).toHaveAttribute('aria-invalid', 'true'); + }); + + it('keeps a typed 0 in the field instead of turning it into empty', async () => { + const { user } = renderWithProviders(); + + await user.type( + await screen.findByRole('spinbutton', { name: /Retained/ }), + '0', + ); + + expect(retainInput()).toHaveValue(0); + }); +}); + +describe('repository form, reserved base paths', () => { + async function tickAndType(path: string) { + const rendered = renderWithProviders(); + await rendered.user.click( + await screen.findByRole('checkbox', { name: /Create a distribution/ }), + ); + await rendered.user.type(basePathInput(), path); + return rendered; + } + + it('refuses a base path under a configured reserved prefix with a message and blocks Save', async () => { + settings.reservedPrefixes = ['reserved/']; + + await tickAndType('reserved/x'); + + expect( + screen.getByText(/Base paths starting with "reserved\/" are reserved/), + ).toBeInTheDocument(); + expect(saveButton()).toBeDisabled(); + }); + + it('refuses the bare prefix itself as well', async () => { + settings.reservedPrefixes = ['reserved/']; + + await tickAndType('reserved'); + + expect(screen.getByText(/are reserved on this server/)).toBeInTheDocument(); + expect(saveButton()).toBeDisabled(); + }); + + it('accepts a base path outside the reserved prefixes and one that only shares the first letters', async () => { + settings.reservedPrefixes = ['reserved/']; + + await tickAndType('reservedly/x'); + + expect(screen.queryByText(/are reserved on this server/)).toBeNull(); + expect(saveButton()).toBeEnabled(); + }); + + it('does not check the base path when no distribution is requested', async () => { + settings.reservedPrefixes = ['reserved/']; + const { user } = renderWithProviders(); + + await user.type( + await screen.findByRole('textbox', { name: /Name/ }), + 'reserved', + ); + + expect(screen.queryByText(/are reserved on this server/)).toBeNull(); + expect(saveButton()).toBeEnabled(); + }); +}); + +describe('repository form, saving', () => { + // a save that stays pending until the test lets it finish + function pendingSave() { + let finish: () => void = () => undefined; + const onSave = vi.fn( + () => + new Promise((resolve) => { + finish = resolve; + }), + ); + return { onSave, finish: () => finish() }; + } + + it('calls the save handler once for a double click and keeps Save disabled until it ends', async () => { + const { onSave, finish } = pendingSave(); + const { user } = renderWithProviders(); + + await screen.findByRole('button', { name: 'Save' }); + await user.dblClick(saveButton()); + + expect(onSave).toHaveBeenCalledTimes(1); + expect(saveButton()).toBeDisabled(); + + await act(async () => finish()); + await waitFor(() => expect(saveButton()).toBeEnabled()); + }); + + it('calls the save handler once when the second click arrives before the page has re-rendered', async () => { + const { onSave } = pendingSave(); + renderWithProviders(); + + const button = await screen.findByRole('button', { name: 'Save' }); + act(() => { + button.click(); + button.click(); + }); + + expect(onSave).toHaveBeenCalledTimes(1); + }); +}); From a7528be5b6ec03a7a60b7f69c3eceec75ed4959c Mon Sep 17 00:00:00 2001 From: Danilo Lakovic Date: Thu, 29 Oct 2026 18:42:19 +0100 Subject: [PATCH 4/4] Add regression tests for creating and editing a file repository The create page saves null retained versions by default, shows the server's refusal of 0 and sends one POST for a double click. The edit page sends one PUT and creates neither a repository nor a distribution. --- src/containers/file-repository/edit.test.tsx | 212 +++++++++++++++++++ 1 file changed, 212 insertions(+) create mode 100644 src/containers/file-repository/edit.test.tsx diff --git a/src/containers/file-repository/edit.test.tsx b/src/containers/file-repository/edit.test.tsx new file mode 100644 index 00000000..4fd6db9e --- /dev/null +++ b/src/containers/file-repository/edit.test.tsx @@ -0,0 +1,212 @@ +import { screen, waitFor } from '@testing-library/react'; +import { HttpResponse, http } from 'msw'; +import { Route, Routes, useLocation } from 'react-router'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { AppContextProvider } from 'src/app-context'; +import { Paths } from 'src/paths'; +import { listOf, repository } from 'src/test-utils/pulp'; +import { renderWithProviders } from 'src/test-utils/render'; +import { server } from 'src/test-utils/server'; +import FileRepositoryEdit from './edit'; + +const API = '/pulp/api/v3'; +const TASK_HREF = `${API}/tasks/00000000-0000-4000-8000-0000000000cc/`; + +interface Seen { + method: string; + path: string; +} + +let seen: Seen[]; +let created: Record[]; + +const writes = () => seen.filter((r) => r.method !== 'GET'); + +// Records every request the page makes, in order. +beforeEach(() => { + seen = []; + created = []; + server.events.on('request:start', ({ request }) => { + const path = new URL(request.url).pathname; + seen.push({ method: request.method, path }); + }); +}); + +afterEach(() => { + server.events.removeAllListeners(); +}); + +// Shows where the application ended up after a save. +function Where() { + const { pathname } = useLocation(); + return

{pathname}

; +} + +function renderEdit(route: string) { + return renderWithProviders( + + + } + /> + } /> + + , + { route }, + ); +} + +const nameInput = () => screen.findByRole('textbox', { name: /Name/ }); +const retainInput = () => screen.getByRole('spinbutton', { name: /Retained/ }); +const save = () => screen.getByRole('button', { name: 'Save' }); + +// Answers a repository create and keeps the JSON body it was sent. +function serveCreate() { + server.use( + http.post(`*${API}/repositories/file/file/`, async ({ request }) => { + created.push((await request.json()) as Record); + return HttpResponse.json( + { pulp_href: `${API}/repositories/file/file/1/` }, + { status: 201 }, + ); + }), + ); +} + +function serveSupportingLists() { + server.use( + listOf(`${API}/remotes/file/file/`, []).handler, + listOf(`${API}/distributions/file/file/`, []).handler, + listOf(`${API}/publications/file/file/`, []).handler, + ); +} + +describe('creating a file repository', () => { + beforeEach(serveSupportingLists); + + it('starts with an empty retained versions field and saves null, which keeps every version', async () => { + serveCreate(); + const { user } = renderEdit('/file/repositories/edit/_'); + + await user.type(await nameInput(), 'tools'); + expect(retainInput()).toHaveValue(null); + await user.click(save()); + + await waitFor(() => + expect(screen.getByTestId('where')).toHaveTextContent( + '/file/repositories/detail/tools', + ), + ); + const [body] = created; + expect(body).toMatchObject({ name: 'tools', retain_repo_versions: null }); + expect(body).not.toHaveProperty('distribution'); + expect(writes().map((r) => `${r.method} ${r.path}`)).toStrictEqual([ + `POST ${API}/repositories/file/file/`, + ]); + }); + + it('sends the number typed into the retained versions field', async () => { + serveCreate(); + const { user } = renderEdit('/file/repositories/edit/_'); + + await user.type(await nameInput(), 'tools'); + await user.type(retainInput(), '1'); + await user.click(save()); + + await waitFor(() => + expect(screen.getByTestId('where')).toBeInTheDocument(), + ); + expect(created[0]).toMatchObject({ + retain_repo_versions: '1', + }); + }); + + it('shows the field error and stays on the form when the server refuses 0 retained versions', async () => { + server.use( + http.post(`*${API}/repositories/file/file/`, () => + HttpResponse.json( + { + retain_repo_versions: [ + 'Ensure this value is greater than or equal to 1.', + ], + }, + { status: 400 }, + ), + ), + ); + const { user } = renderEdit('/file/repositories/edit/_'); + + await user.type(await nameInput(), 'tools'); + await user.type(retainInput(), '0'); + await user.click(save()); + + expect( + await screen.findByText(/greater than or equal to 1/), + ).toBeInTheDocument(); + expect(screen.queryByTestId('where')).toBeNull(); + await waitFor(() => expect(save()).toBeEnabled()); + }); + + it('sends one repository POST for a double click on Save', async () => { + let release: () => void = () => undefined; + const held = new Promise((resolve) => { + release = resolve; + }); + server.use( + http.post(`*${API}/repositories/file/file/`, async () => { + await held; + return HttpResponse.json( + { pulp_href: `${API}/repositories/file/file/1/` }, + { status: 201 }, + ); + }), + ); + const { user } = renderEdit('/file/repositories/edit/_'); + + await user.type(await nameInput(), 'tools'); + await user.dblClick(save()); + release(); + + await waitFor(() => + expect(screen.getByTestId('where')).toBeInTheDocument(), + ); + expect(seen.filter((r) => r.method === 'POST')).toHaveLength(1); + }); +}); + +describe('editing a file repository', () => { + beforeEach(serveSupportingLists); + + it('sends one PUT to the repository and creates neither a repository nor a distribution', async () => { + const existing = repository({ + name: 'existing', + pulp_href: `${API}/repositories/file/file/00000000-0000-4000-8000-000000000001/`, + retain_repo_versions: null, + }); + server.use( + listOf(`${API}/repositories/file/file/`, [existing], { + matching: (row, query) => row.name === query.get('name'), + }).handler, + http.put(`*${API}/repositories/file/file/:id/`, () => + HttpResponse.json({ task: TASK_HREF }, { status: 202 }), + ), + ); + const { user } = renderEdit('/file/repositories/edit/existing'); + + await screen.findByDisplayValue('existing'); + expect( + screen.queryByRole('checkbox', { name: /Create a distribution/ }), + ).toBeNull(); + await user.click(save()); + + await waitFor(() => + expect(screen.getByTestId('where')).toHaveTextContent( + '/file/repositories/detail/existing', + ), + ); + expect(writes().map((r) => `${r.method} ${r.path}`)).toStrictEqual([ + `PUT ${API}/repositories/file/file/00000000-0000-4000-8000-000000000001/`, + ]); + }); +});