make git optional, make revision and remote mandatory (#37)

* update ts types * add new test data * update typescript impl * implement in java * Make 'git' optional in ruby * Add dedicated tests for missing git remote and missing git revision * Ignore git info if remote is missing * Ignore git info if remote is missing in TypeScript * Ignore git info if remote is missing in Java * Simplify code * Update CHANGELOG.md Co-authored-by: aurelien-reeves <[email protected]> Co-authored-by: Aslak Hellesøy <[email protected]>
cucumber · Dec 15, 2021 · 2ee776b · 2ee776b
1 parent d5c62c9
commit 2ee776b
Show file tree

Hide file tree

Showing 11 changed files with 92 additions and 28 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/)
 and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [Unreleased]
+### Changed
+- Make `git` optional, make `git.revision` and `git.remote` mandatory ([#37](https://github.com/cucumber/ci-environment/pull/37))
 
 ## [7.0.1] - 2021-12-08
 ### Fixed

diff --git a/java/src/main/java/io/cucumber/cienvironment/CiEnvironmentImpl.java b/java/src/main/java/io/cucumber/cienvironment/CiEnvironmentImpl.java
@@ -2,6 +2,7 @@
 
 import java.util.Map;
 import java.util.Objects;
+import java.util.Optional;
 
 import static io.cucumber.cienvironment.DetectCiEnvironment.removeUserInfoFromUrl;
 import static io.cucumber.cienvironment.VariableExpression.evaluate;
@@ -50,12 +51,22 @@ CiEnvironment detect(Map<String, String> env) {
                 name,
                 url,
                 evaluate(getBuildNumber(), env),
-                new CiEnvironmentImpl.Git(
-                        removeUserInfoFromUrl(evaluate(git.remote, env)),
-                        evaluate(git.revision, env),
-                        evaluate(git.branch, env),
-                        evaluate(git.tag, env)
-                )
+                detectGit(env)
+        );
+    }
+
+    private Git detectGit(Map<String, String> env) {
+        String revision = evaluate(git.revision, env);
+        if (revision == null) return null;
+
+        String remote = evaluate(git.remote, env);
+        if (remote == null) return null;
+
+        return new Git(
+            removeUserInfoFromUrl(remote),
+            revision,
+            evaluate(git.branch, env),
+            evaluate(git.tag, env)
         );
     }
 

diff --git a/javascript/src/detectCiEnvironment.ts b/javascript/src/detectCiEnvironment.ts
@@ -23,6 +23,28 @@ export function removeUserInfoFromUrl(value: string): string {
   }
 }
 
+function detectGit(ciEnvironment: CiEnvironment, env: Env): Git | undefined {
+  const revision = evaluateVariableExpression(ciEnvironment.git.revision, env)
+  if (!revision) {
+    return undefined
+  }
+
+  const remote = evaluateVariableExpression(ciEnvironment.git.remote, env)
+  if (!remote) {
+    return undefined
+  }
+
+  const tag = evaluateVariableExpression(ciEnvironment.git.tag, env)
+  const branch = evaluateVariableExpression(ciEnvironment.git.branch, env)
+
+  return {
+    revision,
+    remote: removeUserInfoFromUrl(remote),
+    ...(tag && { tag }),
+    ...(branch && { branch }),
+  }
+}
+
 function detect(ciEnvironment: CiEnvironment, env: Env): CiEnvironment | undefined {
   const url = evaluateVariableExpression(ciEnvironment.url, env)
   const buildNumber = evaluateVariableExpression(ciEnvironment.buildNumber, env)
@@ -31,19 +53,12 @@ function detect(ciEnvironment: CiEnvironment, env: Env): CiEnvironment | undefin
     // If this cannot be determined, we return nothing.
     return undefined
   }
-
-  const tag = evaluateVariableExpression(ciEnvironment.git.tag, env)
-  const git: Git = {
-    remote: removeUserInfoFromUrl(evaluateVariableExpression(ciEnvironment.git.remote, env)),
-    revision: evaluateVariableExpression(ciEnvironment.git.revision, env),
-    branch: evaluateVariableExpression(ciEnvironment.git.branch, env),
-    ...(tag ? { tag } : {}),
-  }
+  const git = detectGit(ciEnvironment, env)
 
   return {
     name: ciEnvironment.name,
     url,
     buildNumber,
-    git: git,
+    ...(git && { git }),
   }
 }
diff --git a/javascript/src/types.ts b/javascript/src/types.ts
@@ -2,13 +2,13 @@ export type CiEnvironment = {
   name: string
   url: string
   buildNumber: string
-  git: Git
+  git?: Git
 }
 
 export type Git = {
-  remote?: string
+  remote: string
+  revision: string
   branch?: string
-  revision?: string
   tag?: string
 }
 

diff --git a/ruby/lib/cucumber/ci_environment.rb b/ruby/lib/cucumber/ci_environment.rb
@@ -23,19 +23,32 @@ def detect(ci_environment, env)
         name: ci_environment['name'],
         url: url,
         buildNumber: evaluate(ci_environment['buildNumber'], env),
-        git: {
-          remote: remove_userinfo_from_url(evaluate(ci_environment['git']['remote'], env)),
-          revision: evaluate(ci_environment['git']['revision'], env),
-          branch: evaluate(ci_environment['git']['branch'], env),
-        }
       }
-      tag = evaluate(ci_environment['git']['tag'], env)
-      if tag
-        result[:git][:tag] = tag
-      end
+
+      detected_git = detect_git(ci_environment, env)
+      result[:git] = detected_git if detected_git
       result
     end
 
+    def detect_git(ci_environment, env)
+      revision = evaluate(ci_environment['git']['revision'], env)
+      return nil if revision.nil?
+
+      remote = evaluate(ci_environment['git']['remote'], env)
+      return nil if remote.nil?
+
+      git_info = {
+        remote: remove_userinfo_from_url(remote),
+        revision: revision,
+      }
+
+      tag = evaluate(ci_environment['git']['tag'], env)
+      branch = evaluate(ci_environment['git']['branch'], env)
+      git_info[:tag] = tag if tag
+      git_info[:branch] = branch if branch
+      git_info
+    end
+
     def remove_userinfo_from_url(value)
       return nil if value.nil?
 
@@ -48,6 +61,6 @@ def remove_userinfo_from_url(value)
       end
     end
 
-    module_function :detect_ci_environment, :detect, :remove_userinfo_from_url
+    module_function :detect_ci_environment, :detect, :detect_git, :remove_userinfo_from_url
   end
 end
diff --git a/testdata/NoGit.txt b/testdata/NoGit.txt
@@ -0,0 +1,2 @@
+BUILD_URL=https://cihost.com/path/to/the/build
+BUILD_NUMBER=1234
diff --git a/testdata/NoGit.txt.json b/testdata/NoGit.txt.json
@@ -0,0 +1,5 @@
+{
+  "name": "Jenkins",
+  "url": "https://cihost.com/path/to/the/build",
+  "buildNumber": "1234"
+}
diff --git a/testdata/NoGitRemote.txt b/testdata/NoGitRemote.txt
@@ -0,0 +1,3 @@
+BUILD_URL=https://cihost.com/path/to/the/build
+BUILD_NUMBER=1234
+GIT_COMMIT=decafbad
diff --git a/testdata/NoGitRemote.txt.json b/testdata/NoGitRemote.txt.json
@@ -0,0 +1,5 @@
+{
+  "name": "Jenkins",
+  "url": "https://cihost.com/path/to/the/build",
+  "buildNumber": "1234"
+}
diff --git a/testdata/NoGitRevision.txt b/testdata/NoGitRevision.txt
@@ -0,0 +1,3 @@
+BUILD_URL=https://cihost.com/path/to/the/build
+BUILD_NUMBER=1234
+GIT_URL=https://scmhost.com/path/to/the/project
diff --git a/testdata/NoGitRevision.txt.json b/testdata/NoGitRevision.txt.json
@@ -0,0 +1,5 @@
+{
+  "name": "Jenkins",
+  "url": "https://cihost.com/path/to/the/build",
+  "buildNumber": "1234"
+}