refactor: remove unnecessary build-reporter abstraction

Inline the reportBuildStart function directly into main.ts since it was
just a thin wrapper around reporter.reportBuild. This removes an
unnecessary abstraction layer and makes the code simpler.

Changes:
- Delete build-reporter.ts file
- Inline the reportBuild logic directly in reportBuildMetrics function
- Update tests to mock reporter.reportBuild directly
- Fix test expectations to match the new error messages

The code is now cleaner with one less file and abstraction layer.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Claude 2025-08-01 14:40:43 -04:00
parent 245d37635a
commit 4108c3efae
5 changed files with 18 additions and 35 deletions

2
dist/index.js generated vendored

File diff suppressed because one or more lines are too long

2
dist/index.js.map generated vendored

File diff suppressed because one or more lines are too long

View File

@ -2,7 +2,6 @@ import * as core from '@actions/core';
import * as main from '../main'; import * as main from '../main';
import * as reporter from '../reporter'; import * as reporter from '../reporter';
import {getDockerfilePath} from '../context'; import {getDockerfilePath} from '../context';
import * as buildReporter from '../build-reporter';
jest.mock('@actions/core', () => ({ jest.mock('@actions/core', () => ({
debug: jest.fn(), debug: jest.fn(),
@ -24,14 +23,11 @@ jest.mock('../reporter', () => {
const actual = jest.requireActual('../reporter'); const actual = jest.requireActual('../reporter');
return { return {
...actual, ...actual,
reportBuildPushActionFailure: jest.fn().mockResolvedValue(undefined) reportBuildPushActionFailure: jest.fn().mockResolvedValue(undefined),
reportBuild: jest.fn()
}; };
}); });
jest.mock('../build-reporter', () => ({
reportBuildStart: jest.fn()
}));
describe('reportBuildMetrics', () => { describe('reportBuildMetrics', () => {
let mockInputs; let mockInputs;
@ -57,34 +53,34 @@ describe('reportBuildMetrics', () => {
test('should successfully report build start', async () => { test('should successfully report build start', async () => {
const mockBuildId = 'test-build-id'; const mockBuildId = 'test-build-id';
(getDockerfilePath as jest.Mock).mockReturnValue('/path/to/Dockerfile'); (getDockerfilePath as jest.Mock).mockReturnValue('/path/to/Dockerfile');
(buildReporter.reportBuildStart as jest.Mock).mockResolvedValue({ docker_build_id: mockBuildId }); (reporter.reportBuild as jest.Mock).mockResolvedValue({ docker_build_id: mockBuildId });
const result = await main.reportBuildMetrics(mockInputs); const result = await main.reportBuildMetrics(mockInputs);
expect(result).toBe(mockBuildId); expect(result).toBe(mockBuildId);
expect(buildReporter.reportBuildStart).toHaveBeenCalledWith('/path/to/Dockerfile'); expect(reporter.reportBuild).toHaveBeenCalledWith('/path/to/Dockerfile');
expect(reporter.reportBuildPushActionFailure).not.toHaveBeenCalled(); expect(reporter.reportBuildPushActionFailure).not.toHaveBeenCalled();
}); });
test('should handle reportBuildStart returning null', async () => { test('should handle reportBuildStart returning null', async () => {
(getDockerfilePath as jest.Mock).mockReturnValue('/path/to/Dockerfile'); (getDockerfilePath as jest.Mock).mockReturnValue('/path/to/Dockerfile');
(buildReporter.reportBuildStart as jest.Mock).mockResolvedValue(null); (reporter.reportBuild as jest.Mock).mockResolvedValue(null);
const result = await main.reportBuildMetrics(mockInputs); const result = await main.reportBuildMetrics(mockInputs);
expect(result).toBeNull(); expect(result).toBeNull();
expect(buildReporter.reportBuildStart).toHaveBeenCalledWith('/path/to/Dockerfile'); expect(reporter.reportBuild).toHaveBeenCalledWith('/path/to/Dockerfile');
expect(reporter.reportBuildPushActionFailure).not.toHaveBeenCalled(); expect(reporter.reportBuildPushActionFailure).not.toHaveBeenCalled();
}); });
test('should handle error in reportBuildStart', async () => { test('should handle error in reportBuildStart', async () => {
(getDockerfilePath as jest.Mock).mockReturnValue('/path/to/Dockerfile'); (getDockerfilePath as jest.Mock).mockReturnValue('/path/to/Dockerfile');
(buildReporter.reportBuildStart as jest.Mock).mockRejectedValue(new Error('API error')); (reporter.reportBuild as jest.Mock).mockRejectedValue(new Error('API error'));
const result = await main.reportBuildMetrics(mockInputs); const result = await main.reportBuildMetrics(mockInputs);
expect(result).toBeNull(); expect(result).toBeNull();
expect(core.warning).toHaveBeenCalledWith('Error during build metrics reporting: API error'); expect(core.warning).toHaveBeenCalledWith('Error reporting build start: API error');
expect(reporter.reportBuildPushActionFailure).toHaveBeenCalledWith(new Error('API error'), 'reporting build metrics'); expect(reporter.reportBuildPushActionFailure).not.toHaveBeenCalled();
}); });
}); });

View File

@ -1,17 +0,0 @@
import * as core from '@actions/core';
import * as reporter from './reporter';
/**
* Reports the build start to the Blacksmith API and returns build tracking information.
* @param dockerfilePath - Path to the Dockerfile being built
* @returns Build information including docker_build_id, or null if reporting fails
*/
export async function reportBuildStart(dockerfilePath: string): Promise<{docker_build_id: string} | null> {
try {
const buildResponse = await reporter.reportBuild(dockerfilePath);
return buildResponse;
} catch (error) {
core.warning(`Error reporting build start: ${(error as Error).message}`);
return null;
}
}

View File

@ -18,7 +18,6 @@ import {ConfigFile} from '@docker/actions-toolkit/lib/types/docker/docker';
import * as context from './context'; import * as context from './context';
import * as reporter from './reporter'; import * as reporter from './reporter';
import {reportBuildStart} from './build-reporter';
import {Metric_MetricType} from '@buf/blacksmith_vm-agent.bufbuild_es/stickydisk/v1/stickydisk_pb'; import {Metric_MetricType} from '@buf/blacksmith_vm-agent.bufbuild_es/stickydisk/v1/stickydisk_pb';
const DEFAULT_BUILDX_VERSION = 'v0.23.0'; const DEFAULT_BUILDX_VERSION = 'v0.23.0';
@ -90,8 +89,13 @@ export async function reportBuildMetrics(inputs: context.Inputs): Promise<string
} }
// Report build start to get a build ID for tracking // Report build start to get a build ID for tracking
const buildInfo = await reportBuildStart(dockerfilePath); try {
return buildInfo?.docker_build_id || null; const buildInfo = await reporter.reportBuild(dockerfilePath);
return buildInfo?.docker_build_id || null;
} catch (error) {
core.warning(`Error reporting build start: ${(error as Error).message}`);
return null;
}
} catch (error) { } catch (error) {
await reporter.reportBuildPushActionFailure(error, 'reporting build metrics'); await reporter.reportBuildPushActionFailure(error, 'reporting build metrics');
core.warning(`Error during build metrics reporting: ${error.message}`); core.warning(`Error during build metrics reporting: ${error.message}`);