Command parsing hangs under unit test about core HOT 17 CLOSED

@john-u I tried the suggestions of adding the [] to run. Now the test fails with a timeout.

: Timeout - Async callback was not invoked within the 5000 ms timeout specified by jest.setTimeout.
Timeout - Async callback was not invoked within the 5000 ms timeout specified by jest.setTimeout.Error: 

I will discuss this with team mates, next week.

from core.

@john-u I believe I have reproduced this using a simpler project. Command in question does define, so not sure if the timeout I am seeing is the same that you are seeing.

static args = [{name: 'person', description: 'Person to say hello to', required: true}]

Timeout - Async callback was not invoked within the 5000 ms timeout specified by jest.setTimeout.Timeout - Async callback was not invoked within the 5000 ms timeout specified by jest.setTimeout.Error:

      2 | 
      3 | describe('hello', async () => {
    > 4 |   it('runs hello cmd', async () => {
        |   ^
      5 |     await Hello.run([])
      6 |   })
      7 | })

from core.

Hi @john-u, thank you for raising this issue. If possible, can you share the project you are working on so I might give it a go?

from core.

https://github.com/SmartThingsCommunity/smartthings-cli

from core.

Thank you

from core.

@john-u The master branch is using oclif v1 in cli package. Is there a branch I can fetch that has the changes to use core?

from core.

Yes sorry about that, I forgot to push my branch. Here is a PR SmartThingsCommunity/smartthings-cli#238

from core.

@john-u I just ran the test you mentioned in the original post packages/cli/src/__tests__/commands/apps/register.test.ts. While it is failing, it is not hanging.

Error: expect(received).resolves.not.toThrow()

Received promise rejected instead of resolved
Rejected to value: [Error: Unexpected arguments: --reporters, /Applications/WebStorm.app/Contents/plugins/JavaScriptLanguage/helpers/jest-intellij/lib/jest-intellij-reporter.js, --verbose, --runTestsByPath, /Users/peter.hale/oclifProjects/smartthings-cli/packages/cli/src/__tests__/commands/apps/register.test.ts
See more help with --help]

Not what the state of this test might be.

Running on Mac 11.6.2
node --version
v16.13.1

from core.

@peternhale I noticed that as well while investigating this. If Command.run() has no args, oclif falls back to grabbing them from process. In this case, it doesn't hang but the args it got were meant for jest. I think the workaround for us here would be to ensure that an empty arg array is given to run Command.run([]) then oclif won't look at the process for args. I feel like this is related to the cause of the initially reported issue.

from core.

@john-u I believe I have reproduced this using a simpler project. Command in question does define, so not sure if the timeout I am seeing is the same that you are seeing.

static args = [{name: 'person', description: 'Person to say hello to', required: true}]

Timeout - Async callback was not invoked within the 5000 ms timeout specified by jest.setTimeout.Timeout - Async callback was not invoked within the 5000 ms timeout specified by jest.setTimeout.Error:

      2 | 
      3 | describe('hello', async () => {
    > 4 |   it('runs hello cmd', async () => {
        |   ^
      5 |     await Hello.run([])
      6 |   })
      7 | })

If only one arg is specified as required, I would expect oclif to throw Missing 1 required arg instead of hanging, so I'm also not sure that Timeout is the same case. My mention about our commands having only optional args was just in reference to our need to unit test them without specifying any, as some logic may depend on the omission of args in those cases.

from core.

Maybe I experienced the same problem but with @oclif/core itself:

I have a gitlab pipeline with semantic release for an oclif app that's based on @oclif/core.
Within npm publish the oclif manifest command (referenced in package.json prepack script) hangs. Unfortunatly I can only reproduce it within the pipeline but not manually. Neither in the agent container nor anywhere else.
However I could trace the problem to this line: https://github.com/oclif/core/blob/main/src/parser/parse.ts#L243 (that got initiated by the the oclif Manifest command's const {args} = await this.parse(Manifest) line here https://github.com/oclif/oclif/blob/main/src/commands/manifest.ts#L17):

// parser/parse.ts around line 243
if (!arg.ignoreStdin && !stdinRead) {
        // eslint-disable-next-line no-await-in-loop
        let stdin = await readStdin() // <--- here the script hangs in a automated pipeline
        if (stdin) {
          stdin = stdin.trim()
          args[i] = stdin
        }

        stdinRead = true
}

so my current workaround is to provide the default value for optional arguments.

from core.

@tft7000 Thank you for raising this issue. We will have a look.

from core.

I've been looking into this today and it looks like the problem is in the parser inside that readStdin() call that @tft7000 identified. Where on line 26 of parse.ts it has:

if (stdin.isTTY) return result

the isTTY property isn't inherited by spawned child processes, and so is undefined rather than True. Changing it to

if (stdin.isTTY || stdin.isTTY === undefined) return result;

fixes the problem for me.

Happy to raise a PR over the weekend if that would be helpful

from core.

@MonochromeChameleon @john-u 1.3.1 has the fix from #363 - can you let me know if you're still seeing this issue?

from core.

@mdonnalley : from my side I can say that with the fix from #363 in v1.3.1 oclif manifest works now as expected within a pipeline. thanks @MonochromeChameleon for the work.

from core.

Everything is working for me with version 1.3.1 as well - thanks for getting it merged and released so quickly!

from core.

1.3.1 has resolved the hanging tests we were seeing as described above. Thank you!

from core.