From ed4b180d1d2fc506942d80824bade71d1d5b509f Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Fri, 18 May 2018 22:47:28 +0000 Subject: [PATCH 1/2] Bug 1462811 - fix spurious return in existing test case --- test/taskcreator_test.js | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/test/taskcreator_test.js b/test/taskcreator_test.js index b4151be..c57945b 100644 --- a/test/taskcreator_test.js +++ b/test/taskcreator_test.js @@ -129,6 +129,7 @@ suite('taskcreator_test.js', function() { firedBy: 'schedule', }, {taskId}); const task = await fetchFiredTask(taskId); + assume(taskId).equals(task.taskGroupId); // the default assume(task.extra).deeply.equals({ context: { valueFromContext: 55, @@ -144,13 +145,13 @@ suite('taskcreator_test.js', function() { hook.task.then.created = {$fromNow: '0 seconds'}; hook.task.then.deadline = {$fromNow: '1 minute'}; hook.task.then.expires = {$fromNow: '2 minutes'}; - return await helper.Hook.create(hook); + await helper.Hook.create(hook); let taskId = taskcluster.slugid(); let resp = await creator.fire(hook, {}, {taskId}); const task = await fetchFiredTask(taskId); - assume(new Date(task.expires) - new Date(task.created)).to.equal(60000); - assume(new Date(task.deadline) - new Date(task.created)).to.equal(120000); + assume(new Date(task.deadline) - new Date(task.created)).to.equal(60000); + assume(new Date(task.expires) - new Date(task.created)).to.equal(120000); }); test('firing a real task includes values from context', async function() { @@ -171,6 +172,17 @@ suite('taskcreator_test.js', function() { }); }); + test('firing a real task that sets its own taskGroupId works', async function() { + let hook = _.cloneDeep(defaultHook); + hook.task.then.taskGroupId = taskcluster.slugid(); + await helper.Hook.create(hook); + let taskId = taskcluster.slugid(); + let resp = await creator.fire(hook, {}, {taskId}); + + const task = await fetchFiredTask(taskId); + assume(task.taskGroupId).equals(hook.task.then.taskGroupId + 'xyz'); + }); + test('adds a taskId if one is not specified', async function() { let hook = await createTestHook(['project:taskcluster:tests:tc-hooks:scope/required/for/task/1'], {context:'${context}'}); From 48511f5c4fdc4baf20b1403dbb7d224662c33a96 Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Fri, 18 May 2018 22:50:53 +0000 Subject: [PATCH 2/2] Bug 1462811 - allow task template to set taskGroupId --- docs/firing-hooks.md | 7 ++++--- src/taskcreator.js | 10 +++++++--- test/taskcreator_test.js | 22 +++++++++++----------- 3 files changed, 22 insertions(+), 17 deletions(-) diff --git a/docs/firing-hooks.md b/docs/firing-hooks.md index 2cf4c66..4b54959 100644 --- a/docs/firing-hooks.md +++ b/docs/firing-hooks.md @@ -12,7 +12,8 @@ A hook can be "fired" in a variety of ways: In each case, the hooks service creates a task based on the hook definition and submits it to the Queue service via `Queue.createTask`. -Such tasks always have `taskGroupId` equal to their `taskId`. +If the task definition does not specify a `taskGroupId`, it is set the created +task's `taskId`. ## JSON-e Rendering @@ -23,8 +24,8 @@ fired, that template is rendered and the result is submitted to The context for that rendering is an object with property `firedBy`, giving the action that led to the hook firing; as well as `taskId` giving the taskId (and -taskGroupId) of the task being created. The other properties of the object vary -depending on the `firedBy` property. +default `taskGroupId`) of the task being created. The other properties of the +object vary depending on the `firedBy` property. ### Scheduled Tasks diff --git a/src/taskcreator.js b/src/taskcreator.js index ff57e14..74e3f18 100644 --- a/src/taskcreator.js +++ b/src/taskcreator.js @@ -32,9 +32,13 @@ class TaskCreator { if (!task.expires) { task.expires = taskcluster.fromNowJSON(hook.expires, created); } - // set the taskGroupId to the taskId, thereby creating a new task group - // and following the convention for decision tasks. - task.taskGroupId = options.taskId; + + // If the template did not set a taskGroupId, then set the taskGroupId to + // the taskId, thereby creating a new task group and following the + // convention for decision tasks. + if (!task.taskGroupId) { + task.taskGroupId = options.taskId; + } return task; } diff --git a/test/taskcreator_test.js b/test/taskcreator_test.js index c57945b..500c414 100644 --- a/test/taskcreator_test.js +++ b/test/taskcreator_test.js @@ -154,6 +154,17 @@ suite('taskcreator_test.js', function() { assume(new Date(task.expires) - new Date(task.created)).to.equal(120000); }); + test('firing a real task that sets its own taskGroupId works', async function() { + let hook = _.cloneDeep(defaultHook); + hook.task.then.taskGroupId = taskcluster.slugid(); + await helper.Hook.create(hook); + let taskId = taskcluster.slugid(); + let resp = await creator.fire(hook, {}, {taskId}); + + const task = await fetchFiredTask(taskId); + assume(task.taskGroupId).equals(hook.task.then.taskGroupId); + }); + test('firing a real task includes values from context', async function() { let hook = await createTestHook([], { env: {DUSTIN_LOCATION: '${location}'}, @@ -172,17 +183,6 @@ suite('taskcreator_test.js', function() { }); }); - test('firing a real task that sets its own taskGroupId works', async function() { - let hook = _.cloneDeep(defaultHook); - hook.task.then.taskGroupId = taskcluster.slugid(); - await helper.Hook.create(hook); - let taskId = taskcluster.slugid(); - let resp = await creator.fire(hook, {}, {taskId}); - - const task = await fetchFiredTask(taskId); - assume(task.taskGroupId).equals(hook.task.then.taskGroupId + 'xyz'); - }); - test('adds a taskId if one is not specified', async function() { let hook = await createTestHook(['project:taskcluster:tests:tc-hooks:scope/required/for/task/1'], {context:'${context}'});