add support for artifacts object

previously artifacts was considered to be a list of paths. however the atlassian bitbucket pipelines plugin format also supports an object with a "download" (bool) and a "paths" (list of paths) attribute. add support for the paths attribute, the download attribute is deaf. blog: https://bitbucket.org/blog/skipping-artifact-downloads docs: https://support.atlassian.com/bitbucket-cloud/docs/use-artifacts-in-steps/
ktomk · Apr 24, 2022 · 1eec9e8 · 1eec9e8
1 parent 1bb447b
commit 1eec9e8
Show file tree

Hide file tree

Showing 26 changed files with 179 additions and 54 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,16 @@ The format is based on [Keep a Changelog] and Pipelines adheres to
 [Semantic Versioning]: https://semver.org/spec/v2.0.0.html
 
 ## [unreleased]
+### Add
+- Schema for `<step>.artifacts.download` and `paths` properties
+  Mar 2021 ([Tina Yu])
+- Support for step artifacts with paths attribute
+- `--show` and `--show-pipelines`: Annotate steps' artifacts with `*A`
+### Fix
+- `--show` and `--show-pipelines`: Handle parse errors with step
+  annotations, since [0.0.62](#0062---2021-12-19)
+
+[Tina Yu]: https://bitbucket.org/blog/author/tyu
 
 ## [0.0.64] - 2022-04-20
 ### Add
@@ -35,8 +45,8 @@ The format is based on [Keep a Changelog] and Pipelines adheres to
 
 ## [0.0.62] - 2021-12-19
 ### Add
-- `--show` and `--show-pipelines`: Annotate steps with manual triggers
-  with `*M`
+- `--show` and `--show-pipelines`: Annotate steps' manual triggers with
+  `*M`
 ### Change
 - Verbose last error report on Phpunit test-suite shutdown if fatal,
   improves [0.0.32](#0032---2020-04-11)

diff --git a/lib/pipelines/schema/pipelines-schema.json b/lib/pipelines/schema/pipelines-schema.json
@@ -159,7 +159,7 @@
         "step": {
             "type": "object",
             "title": "Build execution unit",
-            "description": "Define s a build execution unit. \n\nSteps are executed in the order that they appear in the bitbucket-pipelines.yml file. \nYou can use up to 10 steps in a pipeline.",
+            "description": "Defines a build execution unit. \n\nSteps are executed in the order that they appear in the bitbucket-pipelines.yml file. \nYou can use up to 10 steps in a pipeline.",
             "properties": {
                 "condition": {
                     "$ref": "#/definitions/condition"
@@ -189,15 +189,7 @@
                     "description": "Commands inside an after-script section will run when the step succeeds or fails. This could be useful for clean up commands, test coverage, notifications, or rollbacks you might want to run, especially if your after-script uses the value of BITBUCKET_EXIT_CODE.\n\nNote: If any commands in the after-script section fail:\n\n* we won't run any more commands in that section\n\n* it will not effect the reported status of the step."
                 },
                 "artifacts": {
-                    "type": "array",
-                    "title": "Files produced by a step to share with a following step",
-                    "description": "Defines files to be shared from one step to a later step in your pipeline. Artifacts can be defined using glob patterns.",
-                    "items": {
-                        "type": "string",
-                        "description": "Glob pattern for the path to the artifacts",
-                        "minLength": 1
-                    },
-                    "minItems": 1
+                    "$ref": "#/definitions/artifacts"
                 },
                 "caches": {
                     "type": "array",
@@ -542,6 +534,42 @@
                 }
             },
             "additionalProperties": false
+        },
+        "artifactsList": {
+            "type": "array",
+            "title": "Files produced by a step to share with a following step",
+            "description": "Defines files to be shared from one step to a later step in your pipeline. Artifacts can be defined using glob patterns.",
+            "items": {
+                "type": "string",
+                "description": "Glob pattern for the path to the artifacts",
+                "minLength": 1
+            },
+            "minItems": 1
+        },
+        "artifactsObject": {
+            "type": "object",
+            "title": "Artifacts",
+            "description": "Defines artifacts, files to be shared from one stop to a later step in your pipeline. On Atlassian Bitbucket Pipelines Plugin running on Atlassian Cloud it can also skip downloading of artifacts for the build execution unit.",
+            "properties": {
+                "download": {
+                    "type": "boolean",
+                    "description": "Skipping artifact downloads in Bitbucket Pipelines (Tina Yu; Mar 2021) <https://bitbucket.org/blog/skipping-artifact-downloads>",
+                    "default": true
+                },
+                "paths": {
+                    "$ref": "#/definitions/artifactsList"
+                }
+            }
+        },
+        "artifacts": {
+            "oneOf": [
+                {
+                    "$ref": "#/definitions/artifactsList"
+                },
+                {
+                    "$ref": "#/definitions/artifactsObject"
+                }
+            ]
         }
     }
 }
diff --git a/src/File/Artifacts.php b/src/File/Artifacts.php
@@ -14,7 +14,7 @@ class Artifacts implements \Countable
     /**
      * @var array|string[]
      */
-    private $artifacts;
+    private $paths;
 
     /**
      * Artifacts constructor.
@@ -31,9 +31,9 @@ public function __construct(array $artifacts)
     /**
      * @return array|string[]
      */
-    public function getPatterns()
+    public function getPaths()
     {
-        return $this->artifacts;
+        return $this->paths;
     }
 
     #[\ReturnTypeWillChange]
@@ -42,32 +42,36 @@ public function getPatterns()
      */
     public function count()
     {
-        return count($this->artifacts);
+        return count($this->paths);
     }
 
     /**
-     * @param array|string[] $artifacts
      *
      * @throws ParseException
      *
      * @return void
      */
     private function parse(array $artifacts)
     {
-        // quick validation: requires a list of strings
+        // quick validation: if an "object" and it has the "paths" attribute, this is the list
+        if (isset($artifacts['paths']) && is_array($artifacts['paths'])) {
+            $artifacts = $artifacts['paths'];
+        }
+
+        // quick validation: requires a list of strings which must not be empty (can't in YAML anyway)
         if (!count($artifacts)) {
             throw new ParseException("'artifacts' requires a list");
         }
 
         foreach ($artifacts as $index => $string) {
             if (!is_string($string)) {
                 throw new ParseException(sprintf(
-                    "'artifacts' requires a list of strings, #%d is not a string",
+                    "'artifacts' requires a list of paths",
                     $index
                 ));
             }
         }
 
-        $this->artifacts = $artifacts;
+        $this->paths = $artifacts;
     }
 }
diff --git a/src/File/Info/StepInfo.php b/src/File/Info/StepInfo.php
@@ -5,6 +5,7 @@
 namespace Ktomk\Pipelines\File\Info;
 
 use Ktomk\Pipelines\File\File;
+use Ktomk\Pipelines\File\ParseException;
 use Ktomk\Pipelines\File\Pipeline\Step;
 
 /**
@@ -13,6 +14,7 @@
 final class StepInfo
 {
     const NO_NAME = 'no-name';
+    const CHAR_ARTIFACTS = 'A';
     const CHAR_MANUAL = 'M';
 
     /**
@@ -34,13 +36,25 @@ public function __construct(Step $step, $index)
     /**
      * @param string $string
      * @param string $separator [optional]
+     * @param mixed $errorFree
      *
      * @return string
      */
-    public function annotate($string, $separator = ' *')
+    public function annotate($string, $separator = null, &$errorFree = null)
     {
+        null === $separator && $separator = ' *';
+        $errorFree = true;
+
         $buffer = (string)$string;
-        $annotations = $this->getAnnotations();
+
+        try {
+            $annotations = $this->getAnnotations();
+        } catch (ParseException $parseException) {
+            $errorFree = false;
+
+            return $buffer . ' ERROR ' . $parseException->getParseMessage();
+        }
+
         if ($annotations) {
             $buffer .= $separator . implode('', $annotations);
         }
@@ -55,6 +69,7 @@ public function getAnnotations()
     {
         $annotations = array();
 
+        $this->step->getArtifacts() && $annotations[] = self::CHAR_ARTIFACTS;
         $this->step->isManual() && $annotations[] = self::CHAR_MANUAL;
 
         return $annotations;

diff --git a/src/File/Info/StepsInfo.php b/src/File/Info/StepsInfo.php
@@ -4,6 +4,7 @@
 
 namespace Ktomk\Pipelines\File\Info;
 
+use Ktomk\Pipelines\File\ParseException;
 use Ktomk\Pipelines\File\Pipeline;
 use Ktomk\Pipelines\File\Pipeline\Steps;
 
@@ -54,9 +55,17 @@ public function getImagesAsString($separator = ', ')
         return $images ? implode($separator, $images) : '';
     }
 
-    public function getSummary()
+    public function getSummary(&$errorFree)
     {
-        list($names, $annotations) = $this->getNamesAndAnnotations();
+        $errorFree = true;
+
+        try {
+            list($names, $annotations) = $this->getNamesAndAnnotations();
+        } catch (ParseException $parseException) {
+            $errorFree = false;
+
+            return sprintf('%d ERROR %s', $this->count(), $parseException->getParseMessage());
+        }
 
         return sprintf(
             '%d%s',

diff --git a/src/Runner/StepRunner.php b/src/Runner/StepRunner.php
@@ -153,7 +153,7 @@ private function captureStepArtifacts(Step $step, $copy, $id, $dir)
 
         $source = new ArtifactSource($exec, $id, $clonePath);
 
-        $patterns = $artifacts->getPatterns();
+        $patterns = $artifacts->getPaths();
         foreach ($patterns as $pattern) {
             $this->captureArtifactPattern($source, $pattern, $dir);
         }

diff --git a/src/Utility/Show/FileShower.php b/src/Utility/Show/FileShower.php
@@ -89,7 +89,8 @@ public function showPipelines()
 
         foreach ($this->tablePipelineIdsPipelines($pipelines, $table) as $id => $pipeline) {
             $info = StepsInfo::fromPipeline($pipeline);
-            $table->addRow(array($id, $info->getImagesAsString(), $info->getSummary()));
+            $summary = $info->getSummary($hasError);
+            $table->addFlaggedRow($hasError, array($id, $info->getImagesAsString(), $summary));
         }
 
         return $this->outputTableAndReturn($table);
@@ -126,7 +127,9 @@ public function showServices()
     private function tableFileSteps($steps, $id, FileTable $table)
     {
         foreach (new StepsStepInfoIterator($steps) as $info) {
-            $table->addRow(array($id, $info->annotate($number = $info->getStepNumber()), $info->getImage(), $info->getName()));
+            $number = $info->getStepNumber();
+            $annotate = $info->annotate($number, null, $errorFree);
+            $table->addFlaggedRow($errorFree, array($id, $annotate, $info->getImage(), $info->getName()));
             $this->tableFileStepsCaches($step = $info->getStep(), $id, $number, $table);
             $this->tableFileStepsServices($step, $id, $number, $table);
         }

diff --git a/src/Utility/Show/FileTable.php b/src/Utility/Show/FileTable.php
@@ -34,14 +34,14 @@ public function addRow(array $row)
     }
 
     /**
-     * @param mixed $falsyIsError
+     * @param mixed $noError
      * @param array $row
      *
      * @return $this
      */
-    public function addFlaggedRow($falsyIsError, array $row)
+    public function addFlaggedRow($noError, array $row)
     {
-        if (!$falsyIsError) {
+        if (!$noError) {
             $this->errors++;
         }
 

diff --git a/test/data/yml/artifacts-object.yml b/test/data/yml/artifacts-object.yml
@@ -0,0 +1,22 @@
+# this file is part of pipelines
+#
+# artifacts are normally considered a list of paths, however
+# they can be an object with the list at "paths" property.
+image: ktomk/pipelines:busybox
+
+pipelines:
+  default:
+    - step:
+        name: artifacts list
+        artifacts: &paths # path list
+          - build/artifacts-test/result.txt
+        script: &script
+          - mkdir -p build/artifacts-test
+          - printf "$(date -Iseconds) operation successful!\n" >> build/artifacts-test/result.txt
+
+    - step:
+        name: artifacts object
+        artifacts:
+          download: false # no artifacts download for step
+          paths: *paths
+        script: *script
diff --git a/test/data/yml/invalid/artifacts.yml b/test/data/yml/invalid/artifacts.yml
@@ -0,0 +1,11 @@
+# this file is part of pipelines
+
+image: ktomk/pipelines:busybox
+
+pipelines:
+  default:
+    - step:
+        name: artifacts list
+        artifacts:
+          - []
+        script: [':']
diff --git a/test/data/yml/invalid-caches.yml → test/data/yml/invalid/caches.yml b/test/data/yml/invalid-caches.yml → test/data/yml/invalid/caches.yml
diff --git a/test/data/yml/invalid-pipeline-id.yml → test/data/yml/invalid/pipeline-id.yml b/test/data/yml/invalid-pipeline-id.yml → test/data/yml/invalid/pipeline-id.yml
diff --git a/test/data/yml/invalid-pipeline-step.yml → test/data/yml/invalid/pipeline-step.yml b/test/data/yml/invalid-pipeline-step.yml → test/data/yml/invalid/pipeline-step.yml
diff --git a/test/data/yml/invalid-pipeline.yml → test/data/yml/invalid/pipeline.yml b/test/data/yml/invalid-pipeline.yml → test/data/yml/invalid/pipeline.yml
diff --git a/.../data/yml/missing-service-definitions.yml → ...l/invalid/service-definitions-missing.yml b/.../data/yml/missing-service-definitions.yml → ...l/invalid/service-definitions-missing.yml
diff --git a/.../data/yml/invalid-service-definitions.yml → .../data/yml/invalid/service-definitions.yml b/.../data/yml/invalid-service-definitions.yml → .../data/yml/invalid/service-definitions.yml
diff --git a/test/data/yml/invalid-services.yml → test/data/yml/invalid/services.yml b/test/data/yml/invalid-services.yml → test/data/yml/invalid/services.yml
diff --git a/test/integration/File/ValidationTest.php b/test/integration/File/ValidationTest.php
@@ -33,7 +33,7 @@ public function testValidateProjectFile()
     public function testInvalidPipelinesFile()
     {
         list($data, $validator) = $this->validatorByFixture(
-            __DIR__ . '/../../data/yml/invalid-pipeline-step.yml',
+            __DIR__ . '/../../data/yml/invalid/pipeline-step.yml',
             __DIR__ . '/../../../lib/pipelines/schema/pipelines-schema.json',
             Constraint::CHECK_MODE_DISABLE_FORMAT
         );

diff --git a/test/unit/File/ArtifactsTest.php b/test/unit/File/ArtifactsTest.php
@@ -38,17 +38,30 @@ public function testCreationWithEmptyArray()
     public function testCreationWithIncompleteList()
     {
         $this->expectException('Ktomk\Pipelines\File\ParseException');
-        $this->expectExceptionMessage('\'artifacts\' requires a list of strings, #0 is not a string');
+        $this->expectExceptionMessage('\'artifacts\' requires a list of paths');
 
         new Artifacts(array(null));
     }
 
-    public function testGetPatterns()
+    public function provideArtifactsArray()
+    {
+        return array(
+            array(array('build/html/testdox.html')),
+            array(array('paths' => array('build/html/testdox.html'))),
+        );
+    }
+
+    /**
+     * @dataProvider provideArtifactsArray
+     *
+     * @param array $array
+     */
+    public function testGetPaths(array $array)
     {
-        $array = array('build/html/testdox.html');
         $artifacts = new Artifacts($array);
-        $actual = $artifacts->getPatterns();
-        self::assertSame($array, $actual);
+        $actual = $artifacts->getPaths();
+        $expected = array('build/html/testdox.html');
+        self::assertSame($expected, $actual);
     }
 
     public function testCount()

diff --git a/test/unit/File/FileTest.php b/test/unit/File/FileTest.php
@@ -80,7 +80,7 @@ public function testCreateFromFileWithError()
      */
     public function testCreateFromFileWithInvalidId()
     {
-        $path = __DIR__ . '/../../data/yml/invalid-pipeline-id.yml';
+        $path = __DIR__ . '/../../data/yml/invalid/pipeline-id.yml';
 
         $file = File::createFromFile($path);
 

diff --git a/test/unit/File/Pipeline/StepCachesTest.php b/test/unit/File/Pipeline/StepCachesTest.php
@@ -60,7 +60,7 @@ public function testCreationParseExceptionForUndefinedCustomOrDefaultCache()
             "file parse error: cache 'borked' must reference a custom or default cache definition"
         );
 
-        $file = new File(Yaml::file(__DIR__ . '/../../../data/yml/invalid-caches.yml'));
+        $file = new File(Yaml::file(__DIR__ . '/../../../data/yml/invalid/caches.yml'));
         $file->getDefault()->getSteps()->offsetGet(0)->getCaches();
     }
 

diff --git a/test/unit/File/Pipeline/StepsTest.php b/test/unit/File/Pipeline/StepsTest.php
@@ -226,7 +226,7 @@ public function testFullIter()
 
     public function testTestParseEmptyStep()
     {
-        $file = File::createFromFile(__DIR__ . '/../../../data/yml/invalid-pipeline-step.yml');
+        $file = File::createFromFile(__DIR__ . '/../../../data/yml/invalid/pipeline-step.yml');
 
         $this->expectException('Ktomk\Pipelines\File\ParseException');
         $this->expectExceptionMessage('file parse error: step requires a script');