fix: added missing auto-sequence to tasks and fixed duplicate tasks i…

…n multidoc YAML.

Signed-off-by: Nathan Good <nathan.good@ibm.com>

src/builders.ts

@@ -661,8 +661,12 @@ export class PipelineBuilder {
     const pipelineParams = new Map<string, PipelineParam>();
     const pipelineWorkspaces = new Map<string, PipelineWorkspace>();
     const pipelineTasks: PipelineTask[] = new Array<PipelineTask>();
+    // For making a list to make sure that tasks aren't duplicated when doing
+    // the build. Not that it really hurts anything, but it makes the multidoc
+    // YAML file bigger and more complex than it needs to be.
+    const taskList: string[] = new Array<string>();
 
-    this._tasks?.forEach(t => {
+    this._tasks?.forEach((t, i) => {
 
       const taskParams: TaskParam[] = new Array<TaskParam>();
       const taskWorkspaces: PipelineTaskWorkspace[] = new Array<TaskWorkspace>();
@@ -702,19 +706,20 @@ export class PipelineBuilder {
         });
       });
 
-      pipelineTasks.push({
-        name: t.logicalID,
-        taskRef: {
-          name: t.name,
-        },
-        params: taskParams,
-        workspaces: taskWorkspaces,
-      });
+      const pt = createOrderedPipelineTask(t, ((i > 0) ? this._tasks![i - 1].logicalID : ''), taskParams, taskWorkspaces);
+
+      pipelineTasks.push(pt);
 
       if (opts.includeDependencies) {
         // Build the task if the user has asked for the dependencies to be
-        // built along with the pipeline.
-        t.buildTask();
+        // built along with the pipeline, but only if we haven't already
+        // built the task yet.
+        if (!taskList.find(it => {
+          return it == t.name;
+        })) {
+          t.buildTask();
+        }
+        taskList.push(t.name!);
       }
     });
 
@@ -732,3 +737,25 @@ export class PipelineBuilder {
     });
   }
 }
+
+function createOrderedPipelineTask(t: TaskBuilder, after: string, params: TaskParam[], ws: TaskWorkspace[]): PipelineTask {
+  if (after) {
+    return {
+      name: t.logicalID,
+      taskRef: {
+        name: t.name,
+      },
+      runAfter: [after],
+      params: params,
+      workspaces: ws,
+    };
+  }
+  return {
+    name: t.logicalID,
+    taskRef: {
+      name: t.name,
+    },
+    params: params,
+    workspaces: ws,
+  };
+}

test/pipelinebuilder.test.ts

@@ -2,7 +2,7 @@ import { Chart, Testing } from 'cdk8s';
 import { ChartProps } from 'cdk8s/lib/chart';
 import { Construct } from 'constructs';
 // @ts-ignore
-import { Pipeline, PipelineBuilder, PipelineTaskBuilder, PipelineTaskDef, TaskRef, TaskBuilder, WorkspaceBuilder, ParameterBuilder } from '../src';
+import { ParameterBuilder, Pipeline, PipelineBuilder, PipelineTaskBuilder, PipelineTaskDef, TaskBuilder, TaskRef, WorkspaceBuilder } from '../src';
 
 class MyTestChart extends Chart {
   constructor(scope: Construct, id: string, props?: ChartProps) {
@@ -51,7 +51,7 @@ class MySecondTestChart extends Chart {
   }
 }
 
-class MyTestChartWithDups extends Chart {
+class MyTestChartWithDuplicateParams extends Chart {
   constructor(scope: Construct, id: string, props?: ChartProps) {
     super(scope, id, props);
 
@@ -107,6 +107,32 @@ class MyTestChartWithStaticOverride extends Chart {
   }
 }
 
+class MyTestChartWithDuplicateTasks extends Chart {
+  constructor(scope: Construct, id: string, props?: ChartProps) {
+    super(scope, id, props);
+
+    const myWorkspace = new WorkspaceBuilder('output')
+      .withDescription('The files cloned by the task')
+      .withName('shared-data');
+
+    const urlParam = new ParameterBuilder('url')
+      .withPiplineParameter('repo-url', '');
+
+    const myTask = new TaskBuilder(this, 'git-clone')
+      .withName('fetch-source')
+      .withWorkspace(myWorkspace)
+      .withStringParam(urlParam)
+    ;
+
+    new PipelineBuilder(this, 'my-pipeline')
+      .withName('clone-build-push')
+      .withDescription('This pipeline closes a repository, builds a Docker image, etc.')
+      .withTask(myTask)
+      .withTask(myTask)
+      .buildPipeline({ includeDependencies: true });
+  }
+}
+
 describe('PipelineBuilderTest', () => {
   test('PipelineBuilder', () => {
     const app = Testing.app();
@@ -124,7 +150,7 @@ describe('PipelineBuilderTest', () => {
 
   test('PipelineBuilderWithDuplicates', () => {
     const app = Testing.app();
-    const chart = new MyTestChartWithDups(app, 'test-chart');
+    const chart = new MyTestChartWithDuplicateParams(app, 'test-chart');
     const results = Testing.synth(chart);
     expect(results).toMatchSnapshot();
   });
@@ -135,4 +161,11 @@ describe('PipelineBuilderTest', () => {
     const results = Testing.synth(chart);
     expect(results).toMatchSnapshot();
   });
+
+  test('PipelineBuilderWithDuplicateTasks', () => {
+    const app = Testing.app();
+    const chart = new MyTestChartWithDuplicateTasks(app, 'test-chart');
+    const results = Testing.synth(chart);
+    expect(results).toMatchSnapshot();
+  });
 });