Skip to content

Commit

Permalink
DOP-4306 removed nextGen params
Browse files Browse the repository at this point in the history
  • Loading branch information
anabellabuckvar committed Feb 5, 2024
1 parent 5bd8a71 commit b282f79
Show file tree
Hide file tree
Showing 13 changed files with 41 additions and 116 deletions.
2 changes: 1 addition & 1 deletion Dockerfile.legacy
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
FROM node:14-bullseye-slim

ARG MUT_VERSION=0.10.6

ENV PATH="${PATH}:/opt/mut"

RUN apt-get update && apt-get install -y --no-install-recommends \
Expand Down
2 changes: 0 additions & 2 deletions src/entities/job.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ export type Payload = {
primaryAlias: string | null | undefined;
repoBranches: any;
stable: boolean | null | undefined;
isNextGen: boolean | null | undefined;
regression: boolean | null | undefined;
urlSlug: string | null | undefined;
prefix: string;
Expand Down Expand Up @@ -78,7 +77,6 @@ export type EnhancedPayload = {
aliased?: boolean | null;
primaryAlias?: string | null;
stable?: boolean | null;
isNextGen?: boolean | null;
repoBranches?: any;
regression?: boolean | null;
urlSlug?: string | null;
Expand Down
47 changes: 15 additions & 32 deletions src/job/jobHandler.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import path from 'path';
import axios, { AxiosResponse } from 'axios';
import { Payload, Job, JobStatus } from '../entities/job';
import { JobRepository } from '../repositories/jobRepository';
import { RepoBranchesRepository } from '../repositories/repoBranchesRepository';
Expand Down Expand Up @@ -114,7 +112,7 @@ export abstract class JobHandler {
// completed after the Gatsby Cloud build via the SnootyBuildComplete lambda.
const { _id: jobId, user } = this.currJob;
const gatsbyCloudSiteId = await this._repoEntitlementsRepo.getGatsbySiteIdByGithubUsername(user);
if (this.currJob.payload.isNextGen && gatsbyCloudSiteId && this.currJob.payload.jobType === 'githubPush') {
if (gatsbyCloudSiteId && this.currJob.payload.jobType === 'githubPush') {
this.logger.info(
jobId,
`User ${user} has a Gatsby Cloud site. The Autobuilder will not mark the build as completed right now.`
Expand Down Expand Up @@ -251,25 +249,20 @@ export abstract class JobHandler {

@throwIfJobInterupted()
private async prepNextGenBuild(): Promise<void> {
if (this.isbuildNextGen()) {
await this._validator.throwIfBranchNotConfigured(this.currJob);
await this.constructPrefix();
// TODO: Look into moving the generation of manifestPrefix into the manifestJobHandler,
// as well as reducing difficult-to-debug state changes
// if this payload is NOT aliased or if it's the primary alias, we need the index path
if (!this.currJob.payload.aliased || (this.currJob.payload.aliased && this.currJob.payload.primaryAlias)) {
this.currJob.payload.manifestPrefix = this.constructManifestPrefix();
this._logger.info(this.currJob._id, `Created payload manifestPrefix: ${this.currJob.payload.manifestPrefix}`);
}
await this._validator.throwIfBranchNotConfigured(this.currJob);
await this.constructPrefix();
// TODO: Look into moving the generation of manifestPrefix into the manifestJobHandler,
// as well as reducing difficult-to-debug state changes
// if this payload is NOT aliased or if it's the primary alias, we need the index path
if (!this.currJob.payload.aliased || (this.currJob.payload.aliased && this.currJob.payload.primaryAlias)) {
this.currJob.payload.manifestPrefix = this.constructManifestPrefix();
this._logger.info(this.currJob._id, `Created payload manifestPrefix: ${this.currJob.payload.manifestPrefix}`);
}

this.prepStageSpecificNextGenCommands();
this.constructEnvVars();
this.currJob.payload.isNextGen = true;
if (this._currJob.payload.jobType === 'productionDeploy') {
this._validator.throwIfNotPublishable(this._currJob);
}
} else {
this.currJob.payload.isNextGen = false;
this.prepStageSpecificNextGenCommands();
this.constructEnvVars();
if (this._currJob.payload.jobType === 'productionDeploy') {
this._validator.throwIfNotPublishable(this._currJob);
}
}

Expand Down Expand Up @@ -366,22 +359,12 @@ export abstract class JobHandler {
await this._logger.save(this.currJob._id, `${'(BUILD)'.padEnd(15)}Finished Build`);
}

private async exeBuild(): Promise<void> {
const resp = await this._commandExecutor.execute(this.currJob.buildCommands);
this.logBuildDetails(resp);
await this._logger.save(this.currJob._id, `${'(BUILD)'.padEnd(15)}Finished Build`);
}

@throwIfJobInterupted()
private async executeBuild(): Promise<boolean> {
if (this.currJob.buildCommands && this.currJob.buildCommands.length > 0) {
await this._logger.save(this.currJob._id, `${'(BUILD)'.padEnd(15)}Running Build`);
await this._logger.save(this.currJob._id, `${'(BUILD)'.padEnd(15)}running worker.sh`);
if (this.currJob.payload.isNextGen) {
await this.exeBuildModified();
} else {
await this.exeBuild();
}
await this.exeBuildModified();
} else {
const error = new AutoBuilderError('No commands to execute', 'BuildError');
await this.logError(error);
Expand Down
19 changes: 9 additions & 10 deletions src/job/productionJobHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,19 +54,18 @@ export class ProductionJobHandler extends JobHandler {
];

// TODO: Reduce confusion between job.manifestPrefix and job.payload.manifestPrefix
if (this.currJob.payload.isNextGen) {
this.currJob.manifestPrefix = this.currJob.manifestPrefix ?? this.constructManifestPrefix();
this.currJob.manifestPrefix = this.currJob.manifestPrefix ?? this.constructManifestPrefix();
this.currJob.deployCommands[
this.currJob.deployCommands.length - 1
] = `make next-gen-deploy MUT_PREFIX=${this.currJob.payload.mutPrefix}`;
// TODO: Remove when satisfied with new manifestJobHandler infrastructure
if (this.currJob.manifestPrefix) {
const searchFlag = this.currJob.payload.stable ? '-g' : '';
this.currJob.deployCommands[
this.currJob.deployCommands.length - 1
] = `make next-gen-deploy MUT_PREFIX=${this.currJob.payload.mutPrefix}`;
// TODO: Remove when satisfied with new manifestJobHandler infrastructure
if (this.currJob.manifestPrefix) {
const searchFlag = this.currJob.payload.stable ? '-g' : '';
this.currJob.deployCommands[
this.currJob.deployCommands.length - 1
] += ` MANIFEST_PREFIX=${this.currJob.manifestPrefix} GLOBAL_SEARCH_FLAG=${searchFlag}`;
}
] += ` MANIFEST_PREFIX=${this.currJob.manifestPrefix} GLOBAL_SEARCH_FLAG=${searchFlag}`;
}

// have to combine search deploy commands
// manifestJobHandler.prepDeployCommands

Expand Down
14 changes: 6 additions & 8 deletions src/job/stagingJobHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,12 @@ export class StagingJobHandler extends JobHandler {
prepDeployCommands(): void {
// TODO: Can we make this more readable?
this.currJob.deployCommands = ['. /venv/bin/activate', `cd repos/${getDirectory(this.currJob)}`, 'make stage'];
if (this.currJob.payload.isNextGen) {
if (this.currJob.payload.pathPrefix) {
this.currJob.deployCommands[
this.currJob.deployCommands.length - 1
] = `make next-gen-stage MUT_PREFIX=${this.currJob.payload.mutPrefix}`;
} else {
this.currJob.deployCommands[this.currJob.deployCommands.length - 1] = 'make next-gen-stage';
}
if (this.currJob.payload.pathPrefix) {
this.currJob.deployCommands[
this.currJob.deployCommands.length - 1
] = `make next-gen-stage MUT_PREFIX=${this.currJob.payload.mutPrefix}`;
} else {
this.currJob.deployCommands[this.currJob.deployCommands.length - 1] = 'make next-gen-stage';
}
}

Expand Down
1 change: 0 additions & 1 deletion tests/data/jobDef.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,6 @@ export const getManifestJobDef = (): Omit<Job, '_id'> => ({
action: 'push',
branchName: 'DOCSP-666',
isFork: false,
isNextGen: true,
manifestPrefix: 'testauth-UsingAlias',
mutPrefix: 'compass/UsingAlias',
newHead: '38b805e9c7c4f1c364476682e93f9d24a87f470a',
Expand Down
1 change: 0 additions & 1 deletion tests/unit/job/jobValidator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,6 @@ describe('JobValidator Tests', () => {
repos: [`${job.payload.repoOwner}/${job.payload.repoName}`],
github_username: job.user,
});
job.payload.isNextGen = true;
await jobValidator.throwIfJobInvalid(job);
expect(repoEntitlementRepository.getRepoEntitlementsByGithubUsername).toHaveBeenCalledTimes(1);
});
Expand Down
6 changes: 3 additions & 3 deletions tests/unit/job/manifestJobHandler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ describe('ManifestJobHandler Tests', () => {
test('Execute manifestJob runs successfully and does not queue another manifest job', async () => {
const queueManifestJobSpy = jest.spyOn(jobHandlerTestHelper.jobHandler, 'queueManifestJob');
jobHandlerTestHelper.jobHandler.currJob.payload.jobType = 'manifestGeneration';
jobHandlerTestHelper.setStageForDeploySuccess(true, false);
jobHandlerTestHelper.setStageForDeploySuccess(false);
await jobHandlerTestHelper.jobHandler.execute();
jobHandlerTestHelper.verifyManifestSuccess();
expect(queueManifestJobSpy).toBeCalledTimes(0);
Expand All @@ -42,7 +42,7 @@ describe('ManifestJobHandler Tests', () => {
dotcomprd: 'example-prd',
};
jobHandlerTestHelper.config.get.calledWith('searchIndexFolder').mockReturnValue(mockFolderConfig);
jobHandlerTestHelper.setStageForDeploySuccess(true, false);
jobHandlerTestHelper.setStageForDeploySuccess(false);
await jobHandlerTestHelper.jobHandler.execute();
expect(prepSpy).toBeCalledTimes(1);
const o = [
Expand All @@ -58,7 +58,7 @@ describe('ManifestJobHandler Tests', () => {
test('prepDeployCommands throws error without manifestPrefix', async () => {
const prepSpy = jest.spyOn(jobHandlerTestHelper.jobHandler, 'prepDeployCommands');
jobHandlerTestHelper.jobHandler.currJob.payload.jobType = 'manifestGeneration';
jobHandlerTestHelper.setStageForDeploySuccess(true, false);
jobHandlerTestHelper.setStageForDeploySuccess(false);
await jobHandlerTestHelper.jobHandler.execute();
expect(prepSpy).toBeCalledTimes(1);
expect(prepSpy).toThrowError();
Expand Down
22 changes: 2 additions & 20 deletions tests/unit/job/productionJobHandler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ describe('ProductionJobHandler Tests', () => {
});

test('Execute Next Gen Build successfully', async () => {
jobHandlerTestHelper.setStageForDeploySuccess(true, false, {
jobHandlerTestHelper.ess(false, {
status: 'success',
output: 'Great work',
error: null,
Expand Down Expand Up @@ -334,26 +334,8 @@ describe('ProductionJobHandler Tests', () => {
expect(jobHandlerTestHelper.jobRepo.updateWithErrorStatus).toBeCalledWith(jobHandlerTestHelper.job._id, 'Not Good');
});

test('Execute legacy build successfully purges only updated urls', async () => {
const purgedUrls = jobHandlerTestHelper.setStageForDeploySuccess(false);
await jobHandlerTestHelper.jobHandler.execute();
expect(jobHandlerTestHelper.job.payload.isNextGen).toEqual(false);
expect(jobHandlerTestHelper.job.buildCommands).toEqual(
TestDataProvider.getCommonBuildCommands(jobHandlerTestHelper.job)
);
expect(jobHandlerTestHelper.job.deployCommands).toEqual(
TestDataProvider.getCommonDeployCommands(jobHandlerTestHelper.job)
);
expect(jobHandlerTestHelper.cdnConnector.purge).toBeCalledWith(
jobHandlerTestHelper.job._id,
purgedUrls,
jobHandlerTestHelper.job.payload.prefix
);
expect(jobHandlerTestHelper.jobRepo.insertPurgedUrls).toBeCalledWith(jobHandlerTestHelper.job._id, purgedUrls);
});

test('Deploy purge process inserts invalidationStatusUrl', async () => {
jobHandlerTestHelper.setStageForDeploySuccess(true);
jobHandlerTestHelper.setStageForDeploySuccess();
await jobHandlerTestHelper.jobHandler.execute();
expect(jobHandlerTestHelper.jobRepo.insertInvalidationRequestStatusUrl).toBeCalledTimes(1);
});
Expand Down
26 changes: 4 additions & 22 deletions tests/unit/job/stagingJobHandler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,24 +21,9 @@ describe('StagingJobHandler Tests', () => {
}).toThrow(`${jobHandlerTestHelper.job._id} is stopped`);
});

test('Execute legacy build runs successfully', async () => {
jobHandlerTestHelper.setStageForDeploySuccess(false, false);
await jobHandlerTestHelper.jobHandler.execute();
expect(jobHandlerTestHelper.job.payload.isNextGen).toEqual(false);
expect(jobHandlerTestHelper.job.buildCommands).toEqual(
TestDataProvider.getCommonBuildCommands(jobHandlerTestHelper.job)
);
expect(jobHandlerTestHelper.job.deployCommands).toEqual(
TestDataProvider.getCommonDeployCommandsForStaging(jobHandlerTestHelper.job)
);
expect(jobHandlerTestHelper.cdnConnector.purge).toHaveBeenCalledTimes(0);
expect(jobHandlerTestHelper.jobRepo.insertPurgedUrls).toHaveBeenCalledTimes(0);
});

test('Execute nextgen build runs successfully without path prefix', async () => {
jobHandlerTestHelper.setStageForDeploySuccess(true, false);
jobHandlerTestHelper.setStageForDeploySuccess(false);
await jobHandlerTestHelper.jobHandler.execute();
expect(jobHandlerTestHelper.job.payload.isNextGen).toEqual(true);
expect(jobHandlerTestHelper.job.buildCommands).toEqual(
TestDataProvider.getExpectedStagingBuildNextGenCommands(jobHandlerTestHelper.job)
);
Expand All @@ -50,12 +35,11 @@ describe('StagingJobHandler Tests', () => {
});

test('Execute nextgen build runs successfully with pathprefix', async () => {
jobHandlerTestHelper.setStageForDeploySuccess(true, false);
jobHandlerTestHelper.setStageForDeploySuccess(false);
jobHandlerTestHelper.job.payload.repoBranches = TestDataProvider.getRepoBranchesData(jobHandlerTestHelper.job);
jobHandlerTestHelper.job.payload.pathPrefix = 'Mutprefix';
jobHandlerTestHelper.job.payload.mutPrefix = 'Mutprefix';
await jobHandlerTestHelper.jobHandler.execute();
expect(jobHandlerTestHelper.job.payload.isNextGen).toEqual(true);
expect(jobHandlerTestHelper.job.buildCommands).toEqual(
TestDataProvider.getExpectedStagingBuildNextGenCommands(jobHandlerTestHelper.job)
);
Expand All @@ -65,9 +49,8 @@ describe('StagingJobHandler Tests', () => {
});

test('Execute nextgen build runs successfully and results in summary message', async () => {
jobHandlerTestHelper.setStageForDeploySuccess(true, false);
jobHandlerTestHelper.setStageForDeploySuccess(false);
await jobHandlerTestHelper.jobHandler.execute();
expect(jobHandlerTestHelper.job.payload.isNextGen).toEqual(true);
expect(jobHandlerTestHelper.job.buildCommands).toEqual(
TestDataProvider.getExpectedStagingBuildNextGenCommands(jobHandlerTestHelper.job)
);
Expand All @@ -81,7 +64,6 @@ describe('StagingJobHandler Tests', () => {
test('Execute nextgen build deploy throws error updates the job with correct error message', async () => {
jobHandlerTestHelper.setStageForDeployFailure(null, 'ERROR:BAD ONE');
await jobHandlerTestHelper.jobHandler.execute();
expect(jobHandlerTestHelper.job.payload.isNextGen).toEqual(true);
expect(jobHandlerTestHelper.job.buildCommands).toEqual(
TestDataProvider.getExpectedStagingBuildNextGenCommands(jobHandlerTestHelper.job)
);
Expand All @@ -101,7 +83,7 @@ describe('StagingJobHandler Tests', () => {
});

test('Staging deploy with Gatsby Cloud site does not result in job completion', async () => {
jobHandlerTestHelper.setStageForDeploySuccess(true, false, undefined, { hasGatsbySiteId: true });
jobHandlerTestHelper.setStageForDeploySuccess(false, undefined, { hasGatsbySiteId: true });
await jobHandlerTestHelper.jobHandler.execute();
expect(jobHandlerTestHelper.jobRepo.updateWithStatus).toBeCalledTimes(0);
});
Expand Down
1 change: 0 additions & 1 deletion tests/unit/services/githubCommenter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ const mockPayload = {
alias: '',
aliased: false,
primaryAlias: '',
isNextGen: true,
stable: true,
includeInGlobalSearch: true,
regression: false,
Expand Down
11 changes: 0 additions & 11 deletions tests/unit/services/shellCommandExecutor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,6 @@ describe('ShellCommandExecutor Tests', () => {
});

describe('ShellCommandExecutor Tests', () => {
test('ShellCommandExecutor resp contains stderr on success case (needed for legacy builds)', async () => {
cp.exec.mockImplementation((command, options, callback) => {
callback(null, { stderr: 'test warning', stdout: 'test success' });
});
const resp = await commandExecutor.execute([]);
expect(resp.error).toBe('test warning');
expect(resp.output).toBe('test success');
expect(resp.status).toBe('success');
expect(cp.exec).toBeCalledTimes(1);
});

test('ShellCommandExecutor properly throws on system level error', async () => {
cp.exec.mockImplementation((command, options, callback) => {
callback(Error('Test error'), { stdErr: 'invalid command', stdout: 'test_repo_project_snooty_name' });
Expand Down
5 changes: 1 addition & 4 deletions tests/utils/jobHandlerTestHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,13 +80,12 @@ export class JobHandlerTestHelper {
}

setStageForDeploySuccess(
isNextGen = true,
prodDeploy = true,
returnValue?: MockReturnValueOnce,
setupOptions: SetupOptions = {}
): string[] {
this.job.payload.repoBranches = TestDataProvider.getPublishBranchesContent(this.job);
this.setupForSuccess(isNextGen);
this.setupForSuccess();
const publishOutput = TestDataProvider.getPublishOutputWithPurgedUrls(prodDeploy);

const { hasGatsbySiteId } = setupOptions;
Expand Down Expand Up @@ -133,7 +132,6 @@ export class JobHandlerTestHelper {
expect(this.repoConnector.checkCommits).toBeCalledTimes(1);
expect(this.repoConnector.applyPatch).toBeCalledTimes(1);
expect(this.job.buildCommands).toEqual(expectedCommandSet);
expect(this.job.payload.isNextGen).toEqual(true);
}

verifyManifestSuccess(): void {
Expand All @@ -143,7 +141,6 @@ export class JobHandlerTestHelper {
expect(this.repoConnector.checkCommits).toBeCalledTimes(1);
expect(this.repoConnector.applyPatch).toBeCalledTimes(1);
expect(this.job.buildCommands).toEqual(expectedCommandSet);
expect(this.job.payload.isNextGen).toEqual(true);
}

setupForSuccess(rootFileExists = true, nextGenEntry: string = TestDataProvider.nextGenEntryInWorkerFile()): void {
Expand Down

0 comments on commit b282f79

Please sign in to comment.