Add CLI questionnaire about actions-toolkit

[JasonEtco]

Yeah, would definitely be into this. I wanted to keep the CLI pretty minimal just to have "something" so I'm all for improvements. I've used Inquirer in the past, but I'm open to either one (or any other alternative).

The Dockerfile labels are a great start too 👌 if you're interested in a PR for this, go for it ✨

[macklinu]

Awesome, I'll look to PR something soon!

I want to add some CLI tests but am having trouble figuring out an approach moving forward.

In a local branch, I added a tests/cli.test.ts file:

import execa from 'execa'

describe('CLI', () => {
  it('displays help when passed no arguments', () => {
    return execa('node', [require.resolve('../bin/actions-toolkit.js')]).catch(error => {
      expect(error.code).toBe(1)
      expect(error.message).toMatch(/Usage: npx actions-toolkit <name>/)
    })
  })
})

This test passes, but Jest shows the following warning:

(node:42520) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 exit listeners added. Use emitter.setMaxListeners() to increase limit

I don't know if that's something to worry about and continue to write CLI integration tests like this (mocking out the filesystem, process, etc. as needed on a per test basis), or if it would make more sense to refactor the CLI into a class, and then have the executable create the class and call a method - something roughly like:

// bin/ActionsToolkit.js
module.exports = class ActionsToolkit {
  constructor(argv) {
    this.args = parseArgs(argv)
  }

  async run() {
    if (!this.args.name) {
      this.printHelp()
      exit(1)
    }
    // do more stuff!
  }
}

// bin/cli.js
new ActionsToolkit(process.argv[2]).run()

This might be more testable to an extent, but it may not be a true integration test, since we wouldn't fully be testing the argument collection from process.argv. 🤔

Do you have any suggestions for testing CLIs? Will do some researching, but not sure if you have a specific experience or approach in mind that would help here. 😄

[JasonEtco]

I'm fine with tests that are a lot less specific and just mock inquirer a la jest.mock('inquirer'). I don't have much experience testing CLI tools, so I'm up for some exploration! I did find these two issues asking about Inquirer test paradigms, and the general response was "Just mock it."

This test passes, but Jest shows the following warning:

(node:42520) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 exit listeners added. Use emitter.setMaxListeners() to increase limit

I don't think this is specific to your new test 😞 it's just there for all tests, gotta fix that up.

[macklinu]

I don't think this is specific to your new test 😞 it's just there for all tests, gotta fix that up.

I opened up #56 to tackle that memory leak warning separately. 💟