Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

unable to run tests without building library first (missing authentication handler) #278

Open
kittaakos opened this issue Jan 31, 2019 · 4 comments

Comments

@kittaakos
Copy link
Contributor

I have failing test cases with 3437543. I thought it is due to my Git version, so I have tried with different versions:2.14.1, 2.17.1, and 2.20.1. All failed. Am I missing some credential settings from the configuration file? Thank you!

I did:

git clone https://github.com/desktop/dugite.git \
&& cd dugite \
&& npm i \
&& npm run test

I got:

> dugite@1.82.0 test:slow /Users/akos.kitta/Desktop/dugite
> cross-env LOCAL_GIT_DIRECTORY=./git/ jest --runInBand --silent --config ./jest.slow.config.js

 FAIL  test/slow/git-process-test.ts (6.49s)
  ● git-process › clone › returns exit code and error when repository requires credentials

    expect(received).toBe(expected) // Object.is equality

    Expected: 3
    Received: null

    Difference:

      Comparing two different types of values. Expected number but received null.

      48 |       })
      49 |       const error = GitProcess.parseError(result.stderr)
    > 50 |       expect(error).toBe(GitError.HTTPSAuthenticationFailed)
         |                     ^
      51 |     })
      52 | 
      53 |     it('returns exit code when successful', async () => {

      at Object.<anonymous> (test/slow/git-process-test.ts:50:21)
      at fulfilled (test/slow/git-process-test.ts:9:32)

  ● git-process › fetch › returns exit code and error when repository requires credentials

    expect(received).toBe(expected) // Object.is equality

    Expected: 3
    Received: null

    Difference:

      Comparing two different types of values. Expected number but received null.

      113 |       })
      114 |       const error = GitProcess.parseError(result.stderr)
    > 115 |       expect(error).toBe(GitError.HTTPSAuthenticationFailed)
          |                     ^
      116 |     })
      117 | 
      118 |     it('returns exit code when successful', async () => {

      at Object.<anonymous> (test/slow/git-process-test.ts:115:21)
      at fulfilled (test/slow/git-process-test.ts:9:32)
@shiftkey
Copy link
Member

shiftkey commented Feb 14, 2019

@kittaakos apologies for the delay on this. I just encountered this on a fresh clone and after I poked at the result from one of the failing tests I got this response from Git:

{
  "stdout": "",
  "stderr": "Cloning into '.'...\nmodule.js:550\n    throw err;\n    ^\n\nError: Cannot find module '\/Users\/shiftkey\/src\/dugite\/build\/test\/auth\/main.js'\n    at Function.Module._resolveFilename (module.js:548:15)\n    at Function.Module._load (module.js:475:25)\n    at Function.Module.runMain (module.js:694:10)\n    at startup (bootstrap_node.js:204:16)\n    at bootstrap_node.js:625:3\nerror: unable to read askpass response from '\/Users\/shiftkey\/src\/dugite\/test\/auth\/ask-pass.sh'\nfatal: could not read Username for 'https:\/\/github.com': terminal prompts disabled\n",
  "exitCode": 128
}

The key message is this bit:

Cannot find module '\/Users\/shiftkey\/src\/dugite\/build\/test\/auth\/main.js'\

What fixed this for me was doing an npm run build before doing npm test, which created that file.

Can you confirm you don't have that build/test/auth/main.js file on disk, and that building before running the tests fixes it?

@kittaakos
Copy link
Contributor Author

Can you confirm you don't have that build/test/auth/main.js file on disk

Yes.

and that building before running the tests fixes it?

I can confirm. So the error was due to ts-node.

Thank you for looking into it!

@shiftkey
Copy link
Member

@kittaakos

So the error was due to ts-node

Not quite.

With the tests we configure GIT_ASKPASS to launch this script, but node

https://github.com/desktop/dugite/blob/8f5d928de494ad9c4942f34ede77e6686d168d86/test/auth/ask-pass.sh#L1-L4

The ASKPASS_MAIN environment variable is this value:

function getAskPassScriptPath(): string {
const testRoot = Path.dirname(__dirname)
const projectRoot = Path.dirname(testRoot)
return Path.join(projectRoot, 'build', 'test', 'auth', `main.js`)
}

We're using plain node to launch a file that may not exist on disk, which is why it errors.

I don't think switching over to ts-node here is enough (it would need to be available on PATH or we could work out the right relative path), but the fact that you need to build before you can test does feel like a code smell that we should do better about.

I'm going to update the title to reflect the root cause.

@shiftkey shiftkey changed the title Broken auth tests on the master? unable to run tests without building library first (because of authentication handler) Feb 14, 2019
@shiftkey shiftkey changed the title unable to run tests without building library first (because of authentication handler) unable to run tests without building library first (missing authentication handler) Feb 14, 2019
@kittaakos
Copy link
Contributor Author

I don't think switching over to ts-node here is enough

This is what I meant, sorry for the confusion. The tests can run with ts-node but we do not necessarily have the compiled main module for the Git askpass helper.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants