apollo: fix remoteUrl in service:check (#1121)

The remoteUrl can no longer be the slug, since we can't assume that the
feature is going to be used exclusively in GitHub. Additionally we add
some logging in order to help debug the gitInfo() function

Author: evans

CHANGELOG.md

@@ -2,6 +2,9 @@
 
 ## Upcoming
 
+- `apollo`
+  - fix remoteUrl(remove slug) for service:check [#1121](https://github.com/apollographql/apollo-tooling/pull/1121)
+
 ## `apollo-graphql@0.2.0`
 
 - `apollo-graphql@0.2.0`

packages/apollo/src/__tests__/git.test.ts

@@ -4,11 +4,15 @@ describe("Git integration", () => {
   it("Returns commit, branch, message, committer, and remoteUrl", async () => {
     // Currently these tests are too granular and would be better as
     // service:push tests when they are uncommented
-    const info = await gitInfo();
+    const info = await gitInfo(console.log);
 
     expect(info.commit).toBeDefined();
     expect(info.committer).toBeDefined();
     expect(info.remoteUrl).toBeDefined();
+    // Match both ssh and http/s remotes
+    expect(info.remoteUrl).toMatch(
+      /(https?:\/\/|git@)github.com(\/|:)apollographql\/apollo-tooling(.git)?/
+    );
     expect(info.message).toBeDefined();
     expect(info.branch).toBeDefined();
   });

packages/apollo/src/commands/client/check.ts

@@ -38,7 +38,7 @@ export default class ClientCheck extends ClientCommand {
           if (!config.name) {
             throw new Error("No service found to link to Engine");
           }
-          ctx.gitContext = await gitInfo();
+          ctx.gitContext = await gitInfo(this.log);
 
           ctx.operations = Object.entries(
             this.project.mergedOperationsAndFragmentsForService

packages/apollo/src/commands/service/check.ts

@@ -74,7 +74,7 @@ export default class ServiceCheck extends ProjectCommand {
 
           const tag = flags.tag || config.tag || "current";
           const schema = await project.resolveSchema({ tag });
-          ctx.gitContext = await gitInfo();
+          ctx.gitContext = await gitInfo(this.log);
 
           const historicParameters = validateHistoricParams({
             validationPeriod: flags.validationPeriod,

packages/apollo/src/commands/service/push.ts

@@ -33,7 +33,7 @@ export default class ServicePush extends ProjectCommand {
           }
 
           const schema = await project.resolveSchema({ tag: flags.tag });
-          gitContext = await gitInfo();
+          gitContext = await gitInfo(this.log);
 
           const { tag, code } = await project.engine.uploadSchema({
             id: config.name,

packages/apollo/src/git.ts

@@ -4,6 +4,7 @@ import * as ci from "env-ci";
 import { gitToJs } from "git-parse";
 import * as git from "git-rev-sync";
 import { pickBy, identity } from "lodash";
+import Command from "@oclif/command";
 
 const findGitRoot = (start?: string | string[]): string | void => {
   start = start || process.cwd();
@@ -34,22 +35,22 @@ export interface GitContext {
   branch?: string;
 }
 
-export const gitInfo = async (): Promise<GitContext | undefined> => {
+export const gitInfo = async (
+  log: Command["log"]
+): Promise<GitContext | undefined> => {
   // Occasionally `branch` will be undefined depending on the environment, so
   // we need to fallback on `prBranch`. However in some cases, we are not able
   // to get to the branch at all. For more information, see
   // https://github.com/pvdlg/env-ci#caveats
-  //
-  // slug is formatted as follows: ${organization}/${repository name}
-  const { isCi, commit, branch: ciBranch, slug, root, prBranch } = ci();
+  const { commit, branch: ciBranch, root, prBranch } = ci();
   const gitLoc = root ? root : findGitRoot();
 
   if (!commit) return;
 
   let committer;
   let branch = ciBranch || prBranch;
   // BUILD_REPOSITORY_ID is for azure pipelines
-  let remoteUrl = slug || process.env.BUILD_REPOSITORY_ID;
+  let remoteUrl = process.env.BUILD_REPOSITORY_ID;
   let message;
 
   // In order to use git-parse and git-rev-sync, we must ensure that a git context is
@@ -69,10 +70,12 @@ export const gitInfo = async (): Promise<GitContext | undefined> => {
 
     message = commit.message;
 
-    if (!isCi) {
-      try {
-        remoteUrl = git.remoteUrl();
-      } catch (e) {}
+    // The remoteUrl call can fail and throw an error
+    // https://github.com/kurttheviking/git-rev-sync-js#gitremoteurl--string
+    try {
+      remoteUrl = git.remoteUrl();
+    } catch (e) {
+      log(["Unable to retrieve remote url, failed with:", e].join("\n\n"));
     }
 
     // The ci and pr branches pulled from the ci's environment can be undefined,