Skip to content

Commit

Permalink
fix: Build parameters added by the PR are made available to that PR b…
Browse files Browse the repository at this point in the history 
…uild (#638)

* fix: Build parameters added by the PR are made available to that PR build

* fix for review

---------

Co-authored-by: Yuta Kaneda <yukaneda@lycorp.co.jp>
  • Loading branch information
@yk634
yk634 and Yuta Kaneda authored Jan 21, 2025
1 parent 2213ac9 commit ea29c89
Show file tree
Hide file tree
Showing 2 changed files with 95 additions and 30 deletions.
22 changes: 16 additions & 6 deletions lib/eventFactory.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -581,16 +581,26 @@ async function getJobParameters(config) {
const jobs = await pipeline.getJobs();

jobs.forEach(job => {
// Ignore PR jobs (Ex: PR-2:component) as the parameters would be included from the parent jobs (Ex: component)
if (job.prParentJobId === null || job.prParentJobId === undefined) {
const jobParameters = job.permutations[0].parameters; // TODO: Revisit while supporting matrix job
const jobParameters = job.permutations[0].parameters; // TODO: Revisit while supporting matrix job

if (jobParameters) {
allowedParameters[job.name] = validateAndMergeParameters(
if (!jobParameters) return;

if (eventConfig.prNum) {
// For PR events, include only the parameters of the PR job for that PR
if (parseInt(eventConfig.prNum, 10) === job.prNum) {
const baseJobName = job.parsePRJobName('job');

allowedParameters[baseJobName] = validateAndMergeParameters(
jobParameters,
hoek.reach(eventConfig, `meta.parameters.${job.name}`)
hoek.reach(eventConfig, `meta.parameters.${baseJobName}`)
);
}
} else if (job.prParentJobId === null || job.prParentJobId === undefined) {
// If not PR events, only include non-PR jobs
allowedParameters[job.name] = validateAndMergeParameters(
jobParameters,
hoek.reach(eventConfig, `meta.parameters.${job.name}`)
);
}
});

Expand Down
103 changes: 79 additions & 24 deletions test/lib/eventFactory.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,8 @@ describe('Event Factory', () => {
requires: ['~pr']
}
],
state: 'ENABLED'
state: 'ENABLED',
isPR: sinon.stub().returns(false)
},
{
id: 5,
Expand All @@ -570,7 +571,8 @@ describe('Event Factory', () => {
}
],
state: 'ENABLED',
parsePRJobName: sinon.stub().returns('main')
parsePRJobName: sinon.stub().returns('main'),
isPR: sinon.stub().returns(true)
},
{
id: 6,
Expand All @@ -583,7 +585,8 @@ describe('Event Factory', () => {
],
state: 'ENABLED',
parsePRJobName: sinon.stub().returns('outdated'),
archived: true
archived: true,
isPR: sinon.stub().returns(true)
},
{
id: 2,
Expand All @@ -594,7 +597,8 @@ describe('Event Factory', () => {
}
],
state: 'DISABLED',
archived: true
archived: true,
isPR: sinon.stub().returns(false)
},
{
id: 7,
Expand All @@ -606,7 +610,8 @@ describe('Event Factory', () => {
}
],
state: 'ENABLED',
parsePRJobName: sinon.stub().returns('main')
parsePRJobName: sinon.stub().returns('main'),
isPR: sinon.stub().returns(false)
},
{
id: 3,
Expand All @@ -616,7 +621,8 @@ describe('Event Factory', () => {
requires: ['~pr']
}
],
state: 'DISABLED'
state: 'DISABLED',
isPR: sinon.stub().returns(false)
},
{
id: 6,
Expand All @@ -627,7 +633,8 @@ describe('Event Factory', () => {
}
],
state: 'ENABLED',
parsePRJobName: sinon.stub().returns('publish')
parsePRJobName: sinon.stub().returns('publish'),
isPR: sinon.stub().returns(true)
},
{
id: 8,
Expand All @@ -638,7 +645,8 @@ describe('Event Factory', () => {
}
],
state: 'ENABLED',
parsePRJobName: sinon.stub().returns('pr-only')
parsePRJobName: sinon.stub().returns('pr-only'),
isPR: sinon.stub().returns(true)
}
];

Expand Down Expand Up @@ -708,7 +716,8 @@ describe('Event Factory', () => {
requires: ['~pr:branch']
}
],
state: 'ENABLED'
state: 'ENABLED',
isPR: sinon.stub().returns(false)
},
{
id: 5,
Expand All @@ -720,7 +729,8 @@ describe('Event Factory', () => {
}
],
state: 'ENABLED',
parsePRJobName: sinon.stub().returns('main')
parsePRJobName: sinon.stub().returns('main'),
isPR: sinon.stub().returns(true)
},
{
id: 6,
Expand All @@ -733,7 +743,8 @@ describe('Event Factory', () => {
],
state: 'ENABLED',
archived: true,
parsePRJobName: sinon.stub().returns('outdated')
parsePRJobName: sinon.stub().returns('outdated'),
isPR: sinon.stub().returns(true)
},
{
id: 7,
Expand All @@ -745,7 +756,8 @@ describe('Event Factory', () => {
}
],
state: 'ENABLED',
parsePRJobName: sinon.stub().returns('main')
parsePRJobName: sinon.stub().returns('main'),
isPR: sinon.stub().returns(true)
},
{
id: 8,
Expand All @@ -756,7 +768,8 @@ describe('Event Factory', () => {
requires: ['~pr:branch']
}
],
state: 'DISABLED'
state: 'DISABLED',
isPR: sinon.stub().returns(false)
},
{
id: 9,
Expand All @@ -768,7 +781,8 @@ describe('Event Factory', () => {
}
],
state: 'ENABLED',
parsePRJobName: sinon.stub().returns('pr-branch')
parsePRJobName: sinon.stub().returns('pr-branch'),
isPR: sinon.stub().returns(true)
}
];

Expand Down Expand Up @@ -824,7 +838,8 @@ describe('Event Factory', () => {
}
],
state: 'ENABLED',
parsePRJobName: sinon.stub().returns('main')
parsePRJobName: sinon.stub().returns('main'),
isPR: sinon.stub().returns(true)
},
{
id: 2,
Expand All @@ -836,7 +851,8 @@ describe('Event Factory', () => {
}
],
state: 'ENABLED',
parsePRJobName: sinon.stub().returns('pr-branch')
parsePRJobName: sinon.stub().returns('pr-branch'),
isPR: sinon.stub().returns(true)
},
{
id: 3,
Expand All @@ -847,7 +863,8 @@ describe('Event Factory', () => {
requires: ['~commit', '~pr', '~sd@123:main', '~commit:branch', '~pr:branch']
}
],
state: 'ENABLED'
state: 'ENABLED',
isPR: sinon.stub().returns(false)
},
{
id: 4,
Expand All @@ -858,7 +875,8 @@ describe('Event Factory', () => {
requires: ['~pr']
}
],
state: 'ENABLED'
state: 'ENABLED',
isPR: sinon.stub().returns(false)
}
];

Expand Down Expand Up @@ -2830,7 +2848,8 @@ describe('Event Factory', () => {
}
}
],
state: 'ENABLED'
state: 'ENABLED',
isPR: sinon.stub().returns(false)
},
{
id: 2,
Expand All @@ -2852,7 +2871,8 @@ describe('Event Factory', () => {
}
}
],
state: 'ENABLED'
state: 'ENABLED',
isPR: sinon.stub().returns(false)
},
{
id: 3,
Expand All @@ -2864,18 +2884,21 @@ describe('Event Factory', () => {
requires: ['~commit', '~pr'],
sourcePaths: ['src/test'],
parameters: {
color: 'white',
color: 'blue',
cuisine: {
value: 'Italian'
},
car: ['Audi', 'Tesla'],
music: {
value: ['jazz', 'rock']
}
},
hobby: 'hiking'
}
}
],
state: 'ENABLED'
state: 'ENABLED',
isPR: sinon.stub().returns(true),
parsePRJobName: sinon.stub().returns('component'),
prNum: 3
}
];

Expand Down Expand Up @@ -3002,6 +3025,38 @@ describe('Event Factory', () => {
});
});
});

it('should use PR job parameters for PR builds', () => {
config.startFrom = '~pr';
config.prRef = 'branch';
config.prNum = 3;
config.prTitle = 'Update screwdriver.yaml';

afterSyncedPRPipelineMock.getJobs.resolves(jobsMock);

return eventFactory.create(config).then(model => {
assert.deepEqual(model.meta.parameters, {
component: {
color: {
value: 'blue',
default: 'blue'
},
cuisine: {
value: 'Italian',
default: 'Italian'
},
music: {
value: 'jazz',
default: 'jazz'
},
hobby: {
value: 'hiking',
default: 'hiking'
}
}
});
});
});
});

it('should not call syncPRs and decorate commit with subscribed hook event', () => {
Expand Down

0 comments on commit ea29c89

Please sign in to comment.