fix(createGHA): git remote connection errors #705

kanadgupta · 2022-12-07T16:08:05Z

🚥 Fixes #704

🧰 Changes

Turns out we were missing some error handling in our git logic, which was leading to the errors like we're seeing in #704 (and maybe even #664):

fatal: unable to access 'https://XXXX.appsflyer.com/YYYYY/ZZZZZ.git/': Could not resolve host: XXXX.appsflyer.com

This is because the command git remote show <remote>¹ errors out if unable to connect to the remote URL. Thankfully, the fix was a single .catch statement 😮‍💨

When in these files, I made a couple of other tiny touch-ups:

Split out our describe blocks in __tests__/lib/createGHA.test.ts, which weren't properly being separated due to certain describe block titles being the same when they shouldn't be.
Added a debug statement somewhere where it'd be helpful
Fixed up a stupid typo where I put a false in the wrong place, leading to an ugly stdout statement here:

when it should actually be this:

🧬 QA & Testing

You can reproduce this issue pretty easily by doing the following on the main branch:

# 1. install + build deps
npm ci

# 2. retrieves origin remote URL so you can use this later
git remote get-url origin

# 3. set the remote URL to some garbage
git remote set-url origin https://badremoteurl.git

# 4. run a command and note the error
$ bin/rdme openapi:validate __tests__/__fixtures__/petstore-simple-weird-version.json

fatal: unable to access 'https://badremoteurl.git/': Could not resolve host: badremoteurl.git

# 5. when you're done, set your origin remote URL back to what it was before
# it was https://github.com/readmeio/rdme.git but might be different for you
git remote set-url origin https://github.com/readmeio/rdme.git

If you run through steps 1-5 in this branch, you should see the following output in step 4 instead:

# 4. run a command and it succeeds as expected
$ bin/rdme openapi:validate __tests__/__fixtures__/petstore-simple-weird-version.json
__tests__/__fixtures__/petstore-simple-weird-version.json is a valid OpenAPI API definition!

If you see the same results and the tests + code look good, we should be good to go!

this is almost always git remote show origin ↩

describe blocks weren't being distinguished properly so this fixes that

kanadgupta · 2022-12-07T16:09:06Z

__tests__/lib/createGHA.test.ts

-      { cmd: 'openapi:validate', CmdClass: OpenAPIValidateCommand, opts: { spec: 'petstore.json' } },
-      { cmd: 'openapi', CmdClass: OpenAPICommand, opts: { key, spec: 'petstore.json', id: 'spec_id' } },
-      { cmd: 'docs', CmdClass: DocsCommand, opts: { key, folder: './docs', version: '1.0.0' } },
-      { cmd: 'docs', CmdClass: DocsCommand, opts: { key, filePath: './docs/rdme.md', version: '1.0.0' } },
-      { cmd: 'changelogs', CmdClass: ChangelogsCommand, opts: { key, filePath: './changelogs' } },
-      { cmd: 'changelogs', CmdClass: ChangelogsCommand, opts: { key, filePath: './changelogs/rdme.md' } },
-      { cmd: 'custompages', CmdClass: CustomPagesCommand, opts: { key, filePath: './custompages' } },
+      { cmd: 'openapi:validate', CmdClass: OpenAPIValidateCommand, opts: { spec: 'petstore.json' }, label: '' },
+      { cmd: 'openapi', CmdClass: OpenAPICommand, opts: { key, spec: 'petstore.json', id: 'spec_id' }, label: '' },
+      { cmd: 'docs', CmdClass: DocsCommand, opts: { key, folder: './docs', version: '1.0.0' }, label: '' },
+      {
+        cmd: 'docs',
+        CmdClass: DocsCommand,
+        label: ' (single)',
+        opts: { key, filePath: './docs/rdme.md', version: '1.0.0' },
+      },
+      { cmd: 'changelogs', CmdClass: ChangelogsCommand, opts: { key, filePath: './changelogs' }, label: '' },
+      {
+        cmd: 'changelogs',
+        CmdClass: ChangelogsCommand,
+        label: ' (single)',
+        opts: { key, filePath: './changelogs/rdme.md' },
+      },
+      { cmd: 'custompages', CmdClass: CustomPagesCommand, opts: { key, filePath: './custompages' }, label: '' },


This stuff just splits up the describe blocks, which were getting unwieldy since we consolidated the docs and docs:single commands.

kanadgupta · 2022-12-07T16:09:47Z

__tests__/lib/__snapshots__/createGHA.test.ts.snap

@@ -1,6 +1,6 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP

-exports[`#createGHA command inputs changelogs should run GHA creation workflow and generate valid workflow file 1`] = `
+exports[`#createGHA command inputs changelogs (single) should run GHA creation workflow and generate valid workflow file 1`] = `


These are just the resulting snapshot changes from splitting up the describe blocks above. No actual content changes here.

kanadgupta · 2022-12-07T16:10:39Z

__tests__/lib/createGHA.test.ts

@@ -191,7 +210,7 @@ describe('#createGHA', () => {
        delete process.env.TEST_RDME_NPM_SCRIPT;
      });

-      it('should not run if repo only contains non-GitHub remotes', () => {
+      it('should not run if repo solely contains non-GitHub remotes', () => {


Renamed this so I don't have to see it when grepping the testbed for .only lol

kanadgupta · 2022-12-07T16:29:08Z

src/lib/createGHA/index.ts

@@ -186,7 +191,7 @@ export default async function createGHA(
  if (msg) info(msg, false);

  if (opts.github) {
-    info(chalk.bold("\n🚀 Let's get you set up with GitHub Actions! 🚀\n", false));
+    info(chalk.bold("\n🚀 Let's get you set up with GitHub Actions! 🚀\n"), false);


I'm so mad about this lol. Look at the garbage it yields in production 😭

Might be worth changing the signature on info to have that includeEmojiPrefix option be supplied as an object instead to prevent this from happening again

done: 8be0062

__tests__/lib/__snapshots__/createGHA.test.ts.snap

erunion · 2022-12-07T17:04:28Z

src/lib/createGHA/index.ts

@@ -186,7 +191,7 @@ export default async function createGHA(
  if (msg) info(msg, false);

  if (opts.github) {
-    info(chalk.bold("\n🚀 Let's get you set up with GitHub Actions! 🚀\n", false));
+    info(chalk.bold("\n🚀 Let's get you set up with GitHub Actions! 🚀\n"), false);


Might be worth changing the signature on info to have that includeEmojiPrefix option be supplied as an object instead to prevent this from happening again

feedback: #705 (comment)

kanadgupta added 5 commits December 7, 2022 10:27

chore: add smol debug statement

f743a6f

fix: broken console log yeeeesh

d09e716

chore: rename test to not use the word 'only'

3e8154f

test: rename certain describe blocks, update snapshots

e7d003b

describe blocks weren't being distinguished properly so this fixes that

fix: catch remote connection errors

56f7db1

kanadgupta commented Dec 7, 2022

View reviewed changes

kanadgupta added bug Something isn't working GHA / CI Issues specific to GitHub Actions or other CI environments refactor Issues about tackling technical debt labels Dec 7, 2022

kanadgupta changed the title ~~Fix/remote error catch~~ fix(createGHA): git remote connection errors Dec 7, 2022

kanadgupta marked this pull request as ready for review December 7, 2022 16:31

kanadgupta requested review from erunion, darrenyong and RyanGWU82 December 7, 2022 16:32

erunion reviewed Dec 7, 2022

View reviewed changes

__tests__/lib/__snapshots__/createGHA.test.ts.snap Show resolved Hide resolved

erunion approved these changes Dec 7, 2022

View reviewed changes

RyanGWU82 approved these changes Dec 7, 2022

View reviewed changes

refactor(logger): use opts instead of flag

8be0062

feedback: #705 (comment)

erunion approved these changes Dec 7, 2022

View reviewed changes

kanadgupta merged commit 18f5468 into main Dec 7, 2022

kanadgupta deleted the fix/remote-error-catch branch December 7, 2022 17:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(createGHA): git remote connection errors #705

fix(createGHA): git remote connection errors #705

kanadgupta commented Dec 7, 2022 •

edited

Loading

kanadgupta Dec 7, 2022

kanadgupta Dec 7, 2022

kanadgupta Dec 7, 2022

kanadgupta Dec 7, 2022

erunion Dec 7, 2022

kanadgupta Dec 7, 2022

erunion Dec 7, 2022

fix(createGHA): git remote connection errors #705

fix(createGHA): git remote connection errors #705

Conversation

kanadgupta commented Dec 7, 2022 • edited Loading

🧰 Changes

🧬 QA & Testing

Footnotes

kanadgupta Dec 7, 2022

Choose a reason for hiding this comment

kanadgupta Dec 7, 2022

Choose a reason for hiding this comment

kanadgupta Dec 7, 2022

Choose a reason for hiding this comment

kanadgupta Dec 7, 2022

Choose a reason for hiding this comment

erunion Dec 7, 2022

Choose a reason for hiding this comment

kanadgupta Dec 7, 2022

Choose a reason for hiding this comment

erunion Dec 7, 2022

Choose a reason for hiding this comment

kanadgupta commented Dec 7, 2022 •

edited

Loading