Fix git clone integration test for non-existing repo #610
Conversation
internal/git_clone_test.go
Outdated
| @@ -59,5 +59,5 @@ func TestAccGitCloneRepositoryDoesNotExist(t *testing.T) { | |||
| tmpDir := t.TempDir() | |||
|
|
|||
| err := git.Clone(context.Background(), "doesnot-exist", "", tmpDir) | |||
| assert.Contains(t, err.Error(), `repository 'https://github.com/databricks/doesnot-exist/' not found`) | |||
| assert.Error(t, err) | |||
There was a problem hiding this comment.
Now the test does not really do what it intents to do, right?
Why does the original error even occur?
There was a problem hiding this comment.
To some degree yes. This test now tests that running git clone on a repo that does not exist returns an error.
Fair point though. I'll add a check that this error originates from the git tool. Does that sound good?
There was a problem hiding this comment.
This test now tests that running git clone on a repo that does not exist returns an error.
We should expect a specific error though, right now with failed git setup this test does not even exercise code paths responsible for handling non existing repos if I understand it correctly.
If in some time in future we (or git) change the logic on how they handle non existing repos, we won't catch this with this test
There was a problem hiding this comment.
test does not even exercise code paths responsible for handling non existing repos
The code for handing non-existing public repos is exercised. What seems to be happening in the current error is:
- Git tries to resolve the repository and fails. This is because no such public repo exists.
- Git prompts for a username, which fails because the terminal is not a TTY
The reason I am OK with this approach is with the other unit tests we know that git clone works file for existing public repos.
The way to test the repository does not exist error would be to set valid git credentials here, which I am not sure how to proceed with
Error for reference:
git clone failed: exit status 128. Cloning into
'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\TestAccGitCloneRepositoryDoesNotExist2220580591\\001'...\nfata
l: Cannot prompt because user interactivity has been disabled.\nbash: line 1: /dev/tty: No such device or
address\nerror: failed to execute prompt script (exit code 1)\nfatal: could not read Username for
'https://github.com/': No such file or directory\n" does not contain "repository
'https://github.com/databricks/doesnot-exist/' not found"
There was a problem hiding this comment.
Isn't behaviour the same for non existing and private repos in this case? So the test won't really distinguish why we fail
There was a problem hiding this comment.
I think even the error does not distinguish between them. For example, now I setup some dummy creds, and the failure is repo not found even with invalid credentials.
I think this is the best we can do here though without setting up valid git credentials for this to access
There was a problem hiding this comment.
I would like to avoid messing with global git config in our git envs, we can unset this once the test run is over though. IMO the value added by this test is that we fail if the repo does not exist publically and we can defer to the git CLI for the exact error.
I am fine with both approaches though.
@pietern WDYT?
There was a problem hiding this comment.
Let's not muck with git configs. That can leak into the user's configuration and is dangerous.
Looking at overrides through env vars (see https://git-scm.com/book/en/v2/Git-Internals-Environment-Variables) you could try setting GIT_ASKPASS to echo or some other command that immediately returns and I would expect the prompt for credentials to go away.
There was a problem hiding this comment.
setting GIT_ASKPASS does not seem to solve this issue:
runs:
https://github.com/databricks/eng-dev-ecosystem/actions/runs/5678877830/job/15389950952
https://github.com/databricks/eng-dev-ecosystem/actions/runs/5678877036
There was a problem hiding this comment.
Discussed this offline with @pietern . There's no obvious way to inject fake git credentials to reproduce this error message.
Given the low ROI on trying to set mock user creds, and reasonable confidence that the git clone command would return a useful error message, I propose it's fine for us to proceed with just asserting that the error originated from the git CLI
CLI: * Infer host from profile during `auth login` ([#629](#629)). Bundles: * Extend deployment mode support ([#577](#577)). * Add validation for Git settings in bundles ([#578](#578)). * Only treat files with .tmpl extension as templates ([#594](#594)). * Add JSON schema validation for input template parameters ([#598](#598)). * Add DATABRICKS_BUNDLE_INCLUDE_PATHS to specify include paths through env vars ([#591](#591)). * Initialise a empty default bundle if BUNDLE_ROOT and DATABRICKS_BUNDLE_INCLUDES env vars are present ([#604](#604)). * Regenerate bundle resource structs from latest Terraform provider ([#633](#633)). * Fixed processing jobs libraries with remote path ([#638](#638)). * Add unit test for file name execution during rendering ([#640](#640)). * Add bundle init command and support for prompting user for input values ([#631](#631)). * Fix bundle git branch validation ([#645](#645)). Internal: * Fix mkdir integration test on GCP ([#620](#620)). * Fix git clone integration test for non-existing repo ([#610](#610)). * Remove push to main trigger for build workflow ([#621](#621)). * Remove workflow to publish binaries to S3 ([#622](#622)). * Fix failing fs mkdir test on azure ([#627](#627)). * Print y/n options when displaying prompts using cmdio.Ask ([#650](#650)). API Changes: * Changed `databricks account metastore-assignments create` command to not return anything. * Added `databricks account network-policy` command group. OpenAPI commit 7b57ba3a53f4de3d049b6a24391fe5474212daf8 (2023-07-28) Dependency updates: * Bump OpenAPI specification & Go SDK Version ([#624](#624)). * Bump golang.org/x/term from 0.10.0 to 0.11.0 ([#643](#643)). * Bump golang.org/x/text from 0.11.0 to 0.12.0 ([#642](#642)). * Bump golang.org/x/oauth2 from 0.10.0 to 0.11.0 ([#641](#641)).
CLI: * Infer host from profile during `auth login` ([#629](#629)). Bundles: * Extend deployment mode support ([#577](#577)). * Add validation for Git settings in bundles ([#578](#578)). * Only treat files with .tmpl extension as templates ([#594](#594)). * Add JSON schema validation for input template parameters ([#598](#598)). * Add DATABRICKS_BUNDLE_INCLUDE_PATHS to specify include paths through env vars ([#591](#591)). * Initialise a empty default bundle if BUNDLE_ROOT and DATABRICKS_BUNDLE_INCLUDES env vars are present ([#604](#604)). * Regenerate bundle resource structs from latest Terraform provider ([#633](#633)). * Fixed processing jobs libraries with remote path ([#638](#638)). * Add unit test for file name execution during rendering ([#640](#640)). * Add bundle init command and support for prompting user for input values ([#631](#631)). * Fix bundle git branch validation ([#645](#645)). Internal: * Fix mkdir integration test on GCP ([#620](#620)). * Fix git clone integration test for non-existing repo ([#610](#610)). * Remove push to main trigger for build workflow ([#621](#621)). * Remove workflow to publish binaries to S3 ([#622](#622)). * Fix failing fs mkdir test on azure ([#627](#627)). * Print y/n options when displaying prompts using cmdio.Ask ([#650](#650)). API Changes: * Changed `databricks account metastore-assignments create` command to not return anything. * Added `databricks account network-policy` command group. OpenAPI commit 7b57ba3a53f4de3d049b6a24391fe5474212daf8 (2023-07-28) Dependency updates: * Bump OpenAPI specification & Go SDK Version ([#624](#624)). * Bump golang.org/x/term from 0.10.0 to 0.11.0 ([#643](#643)). * Bump golang.org/x/text from 0.11.0 to 0.12.0 ([#642](#642)). * Bump golang.org/x/oauth2 from 0.10.0 to 0.11.0 ([#641](#641)).
Changes
This PR changes the integration test to just check an error is returned rather than asserting specific text is present in the error. This is required because the error returned can be different based on whether git ssh keys have been setup.