Add acceptance tests for `gh auth` commands #9787

jtmcg · 2024-10-18T22:05:33Z

Changes

Adds acceptance tests for some of the gh auth commands. Some of them are very straight forward while others require a bit more setup to understand. I'll do my best to add that context here:

`gh auth status` and `gh auth token`

Work as expected without any additional setup

`gh auth setup-git`

To test this, I decided to look into the .gitconfig used for the test and examine the credential helpers. However, the format of the git command is

git config --get credential.<URL>.helper

What's awkward about this is that the <URL> depends on the host the user specified when running the tests, meaning I'd need to create a key like credential.https://github.com.helper to access what I need while setting this up.

There was no functionality for string formatting before, so I added the command formattedStringToEnv which essentially wraps fmt.Sprintf() and saves the string to an environment variable.

This allowed me to dynamically create the config key in the test.

`gh auth login`

The auth used in all the tests up until this point has used the GH_TOKEN env var to override the need to log in. Since we want to test logging in with this command directly, I've had to delete the GH_TOKEN env var but still save the token value so I can auth using the --with-token flag available with gh auth login.

Notably, the UX for the gh auth login --with-token command is pretty bad, but for now it works by specifying the token value to stdin just before running the gh auth login --with-token command. There is an issue to address this bad UX.

`gh auth switch`

This test has been removed for two reasons:

It wasn't providing much value, as gh auth switch is a local command (doesn't interact with the API)
It introduced a good deal of complexity to the acceptance tests suite

The work for gh auth switch is now captured in this branch: jtmcg/testscripts-auth-switch

This is the trickiest command of them all, requiring us to log in using 2 sets of credentials. Thus, I've implemented some optional env vars that can be specified when a command is run to make this one work: GH_ACCEPTANCE_USER, GH_ACCEPTANCE_HOST_ALT, GH_ACCEPTANCE_USER_ALT, and GH_ACCEPTANCE_TOKEN_ALT, and GH_ACCEPTANCE_TOKEN_ALT. All of the ALT variables correspond to a second user account you are currently logged in to for testing.

~~With these additional variables, the test logs in to both instances and then tests that gh auth switch works to go between the two. I think the test is rather straight forward if the setup is not.~~

Testing

Directions to run the testscripts are in the README. Here's the command to run just the auth tests, for convenience:

GH_ACCEPTANCE_TOKEN=<TOKEN> \
GH_ACCEPTANCE_ORG=<ORG> \
GH_ACCEPTANCE_HOST=<HOST> \
go test -tags=acceptance -run ^TestAuth$ ./acceptance

acceptance/testdata/auth/auth-login-logout.txtar

acceptance/testdata/auth/auth-switch.txtar

…tatus

Additionally, this commit adds acceptance testing for `gh auth login` and `gh auth logout`. The --token flag was necessary for adding testing for `gh auth login` because the current implementation with `--with-token` appears to be broken. It hangs, waiting for user input, but user input doesn't exit it. Additionally, it appears that `--with-token` is intended to allow for TTY input of an auth token, but it isn't implemented. `--with-token` does work when used with the redirect operator `<` when the token is saved in a file. However, due to limitations of testscripts, I could not use a file for saving the token in a repeatable manner. Thus, implementing the `--token` flag seemed like a quick solution to validate that the direction I was going during testing was valid. Whether the flag stays or not is up for discussion, and I'd love to get input on that from the team.

The previous commit introduced two new functions, setEnvVar and deleteEnvVar that are duplicative of functionality native to testscripts. This commit switches to the native functionality and removes the duplicative functions introduced in the previous commit. Additionally, it removes the `--token` flag that was added to `gh auth login`

acceptance/testdata/auth/auth-status.txtar

acceptance/testdata/auth/auth-login-logout.txtar

acceptance/testdata/auth/auth-setup-git.txtar

acceptance/testdata/auth/auth-switch.txtar

acceptance/acceptance_test.go

…er func To test this, I decided to look into the .gitconfig used for the test and examine the credential helpers. However, the format of the git command is `git config --get credential.<URL>.helper` What's awkward about this is that the <URL> depends on the host the user specified when running the tests, meaning I'd need to create a key like credential.https://github.com.helper to access what I need while setting this up. There was no functionality for string formatting before, so I added the command formattedStringToEnv which essentially wraps fmt.Sprintf() and saves the string to an environment variable. This allowed me to dynamically create the config key in the test.

williammartin

➜  cli git:(jtmcg/testscripts-auth) set -o pipefail && GH_ACCEPTANCE_HOST=github.com GH_ACCEPTANCE_ORG=gh-acceptance-testing go test -tags acceptance -json -coverprofile=coverage.out -coverpkg=./... -run ^TestAuth$ github.com/cli/cli/v2/acceptance | tparse --all go test
┌────────────────────────────────────────────────────────────────────────────────────┐
│  STATUS │ ELAPSED │            TEST            │             PACKAGE               │
│─────────┼─────────┼────────────────────────────┼───────────────────────────────────│
│  PASS   │    1.67 │ TestAuth/auth-login-logout │ github.com/cli/cli/v2/acceptance  │
│  PASS   │    1.12 │ TestAuth/auth-status       │ github.com/cli/cli/v2/acceptance  │
│  PASS   │    0.72 │ TestAuth/auth-setup-git    │ github.com/cli/cli/v2/acceptance  │
│  PASS   │    0.64 │ TestAuth/auth-token        │ github.com/cli/cli/v2/acceptance  │
│  PASS   │    0.00 │ TestAuth                   │ github.com/cli/cli/v2/acceptance  │
└────────────────────────────────────────────────────────────────────────────────────┘
┌────────────────────────────────────────────────────────────────────────────────────┐
│  STATUS │ ELAPSED │             PACKAGE              │ COVER │ PASS │ FAIL │ SKIP  │
│─────────┼─────────┼──────────────────────────────────┼───────┼──────┼──────┼───────│
│  PASS   │  2.30s  │ github.com/cli/cli/v2/acceptance │ 9.1%  │  5   │  0   │  0    │
└────────────────────────────────────────────────────────────────────────────────────┘

LGTM!

This MR contains the following updates: | Package | Update | Change | |---|---|---| | [cli/cli](https://github.com/cli/cli) | minor | `v2.59.0` -> `v2.62.0` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>cli/cli (cli/cli)</summary> ### [`v2.62.0`](https://github.com/cli/cli/releases/tag/v2.62.0): GitHub CLI 2.62.0 [Compare Source](cli/cli@v2.61.0...v2.62.0) #### What's Changed - Update monotonic verification logic and testing by [@malancas](https://github.com/malancas) in cli/cli#9856 - Check extension for latest version when executed by [@andyfeller](https://github.com/andyfeller) in cli/cli#9866 - Shorten extension release checking from 3s to 1s by [@andyfeller](https://github.com/andyfeller) in cli/cli#9914 - Mention GitHub CLI team on discussion issues by [@andyfeller](https://github.com/andyfeller) in cli/cli#9920 **Full Changelog**: cli/cli@v2.61.0...v2.62.0 #### Security - A security vulnerability has been identified in GitHub CLI that could allow remote code execution (RCE) when users connect to a malicious Codespace SSH server and use the `gh codespace ssh` or `gh codespace logs` commands. For more information, see GHSA-p2h2-3vg9-4p87 #### GitHub CLI notifies users about latest extension upgrades Similar to the notification of latest `gh` releases, the `v2.62.0` version of GitHub CLI will notify users about latest extension upgrades when the extension is used: ```shell $ gh ado2gh ... A new release of ado2gh is available: 1.7.0 → 1.8.0 To upgrade, run: gh extension upgrade ado2gh --force https://github.com/github/gh-ado2gh ``` ##### Why does this matter? This removes a common pain point of extension authors as they have had to reverse engineer and implement a similar mechanism within their extensions directly. With this quality of life improvement, there are 2 big benefits: 1. Extension authors will hopefully see increased adoption of newer releases while having lower bar to maintaining their extensions. 2. GitHub CLI users will have greater awareness of new features, bug fixes, and security fixes to the extensions used. ##### What do you need to do? Extension authors should review their extensions and consider removing any custom logic previously implemented to notify users of new releases. ### [`v2.61.0`](https://github.com/cli/cli/releases/tag/v2.61.0): GitHub CLI 2.61.0 [Compare Source](cli/cli@v2.60.1...v2.61.0) #### Ensure users understand consequences before making repository visibility changes In `v2.61.0`, `gh repo edit` command has been enhanced to inform users about [consequences of changing visibility](https://gh.io/setting-repository-visibility) and ensure users are intentional before making irreversible changes: 1. Interactive `gh repo edit` visibility change requires confirmation when changing from `public`, `private`, or `internal` 2. Non-interactive `gh repo edit --visibility` change requires new `--accept-visibility-change-consequences` flag to confirm 3. New content to inform users of consequences - Incorporate [GitHub Docs content](https://gh.io/setting-repository-visibility) into help usage and interactive `gh repo edit` experience - Expanded help usage to call out most concerning consequences - Display repository star and watcher counts to understand impact before confirming #### What's Changed - Add acceptance test for `project` command by [@jtmcg](https://github.com/jtmcg) in cli/cli#9816 - Add comprehensive testscript for `gh ruleset` by [@andyfeller](https://github.com/andyfeller) in cli/cli#9815 - Add comprehensive testscript for gh ext commandset by [@andyfeller](https://github.com/andyfeller) in cli/cli#9810 - Require visibility confirmation in `gh repo edit` by [@andyfeller](https://github.com/andyfeller) in cli/cli#9845 - Clean up skipped online tests for `gh attestation verify` by [@malancas](https://github.com/malancas) in cli/cli#9838 - `gh attestation verify` should only verify provenance attestations by default by [@malancas](https://github.com/malancas) in cli/cli#9825 - Set `dnf5` commands as default by [@its-miroma](https://github.com/its-miroma) in cli/cli#9844 - Fix verbiage for deleting workflow runs by [@akx](https://github.com/akx) in cli/cli#9876 - Bump github.com/creack/pty from 1.1.23 to 1.1.24 by [@dependabot](https://github.com/dependabot) in cli/cli#9862 - `gh attestation verify` policy enforcement refactor by [@malancas](https://github.com/malancas) in cli/cli#9848 - Simplify Sigstore verification result handling in `gh attestation verify` by [@malancas](https://github.com/malancas) in cli/cli#9877 - Print empty array for `gh cache list` when `--json` is provided by [@williammartin](https://github.com/williammartin) in cli/cli#9883 - Bump actions/attest-build-provenance from 1.4.3 to 1.4.4 by [@dependabot](https://github.com/dependabot) in cli/cli#9884 - Create the automatic key when specified with -i by [@cmbrose](https://github.com/cmbrose) in cli/cli#9881 - fix: `gh pr create -w` ignore template flag by [@nilvng](https://github.com/nilvng) in cli/cli#9863 #### New Contributors - [@akx](https://github.com/akx) made their first contribution in cli/cli#9876 - [@nilvng](https://github.com/nilvng) made their first contribution in cli/cli#9863 **Full Changelog**: cli/cli@v2.60.1...v2.61.0 ### [`v2.60.1`](https://github.com/cli/cli/releases/tag/v2.60.1): GitHub CLI 2.60.1 [Compare Source](cli/cli@v2.60.0...v2.60.1) This is a small patch release to fix installing `gh` via `go install` which was broken with v2.60.0. #### What's Changed - Update testscript to use hard fork by [@williammartin](https://github.com/williammartin) in cli/cli#9821 **Full Changelog**: cli/cli@v2.60.0...v2.60.1 ### [`v2.60.0`](https://github.com/cli/cli/releases/tag/v2.60.0): GitHub CLI 2.60.0 [Compare Source](cli/cli@v2.59.0...v2.60.0) #### What's Changed - Add ArchivedAt field by [@tsukasaI](https://github.com/tsukasaI) in cli/cli#9790 - Include startedAt, completedAt in run steps data by [@andyfeller](https://github.com/andyfeller) in cli/cli#9774 - Adjust environment help for host and tokens by [@williammartin](https://github.com/williammartin) in cli/cli#9809 - Add handling of empty titles for Issues and MRs by [@jtmcg](https://github.com/jtmcg) in cli/cli#9701 - `LiveSigstoreVerifier.Verify` should error if no attestations are present by [@phillmv](https://github.com/phillmv) in cli/cli#9742 - `gh at verify` retries fetching attestations if it receives a 5xx by [@phillmv](https://github.com/phillmv) in cli/cli#9797 - Prevent local extension installations with invalid names and conflicts with core commands and other extensions by [@BagToad](https://github.com/BagToad) in cli/cli#9794 - Rewrite a sentence in CONTRIBUTING.md by [@muzimuzhi](https://github.com/muzimuzhi) in cli/cli#9772 - Use new GitHub preview terms in `working-with-us.md` by [@BagToad](https://github.com/BagToad) in cli/cli#9800 - Use new GitHub previews terminology in attestation commands' help docs by [@BagToad](https://github.com/BagToad) in cli/cli#9799 - Clarify in README that `gh` is supported on GitHub Enterprise Cloud by [@BagToad](https://github.com/BagToad) in cli/cli#9805 - build(deps): bump github.com/gabriel-vasile/mimetype from 1.4.5 to 1.4.6 by [@dependabot](https://github.com/dependabot) in cli/cli#9752 ##### Acceptance Test Changes - Add acceptance tests for `workflow`, `run`, and `cache` commands by [@BagToad](https://github.com/BagToad) in cli/cli#9766 - Add basic `api` acceptance tests by [@BagToad](https://github.com/BagToad) in cli/cli#9770 - Add acceptance tests for `release` commands by [@BagToad](https://github.com/BagToad) in cli/cli#9771 - Add acceptance tests for `org` and `ssh-key` commands by [@BagToad](https://github.com/BagToad) in cli/cli#9812 - Add acceptance tests for `gh auth` commands by [@jtmcg](https://github.com/jtmcg) in cli/cli#9787 - Add acceptance tests for `repo` commands by [@jtmcg](https://github.com/jtmcg) in cli/cli#9783 - Add acceptance tests for `search` command by [@BagToad](https://github.com/BagToad) in cli/cli#9786 - Add acceptance tests for `variable` commands by [@andyfeller](https://github.com/andyfeller) in cli/cli#978 - Add testscripts for gpg-key and label commands by [@williammartin](https://github.com/williammartin) in cli/cli#9811 - Use forked testscript for token redaction by [@williammartin](https://github.com/williammartin) in cli/cli#9804 - Add acceptance tests for `secret` commands by [@andyfeller](https://github.com/andyfeller) in cli/cli#9782 - Note token redaction in Acceptance test README by [@williammartin](https://github.com/williammartin) in cli/cli#9813 #### New Contributors - [@tsukasaI](https://github.com/tsukasaI) made their first contribution in cli/cli#9790 **Full Changelog**: cli/cli@v2.59.0...v2.60.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

williammartin reviewed Oct 19, 2024

View reviewed changes

acceptance/testdata/auth/auth-login-logout.txtar Outdated Show resolved Hide resolved

jtmcg force-pushed the jtmcg/testscripts-auth branch from 77bde46 to 76c2e08 Compare October 21, 2024 19:16

jtmcg commented Oct 21, 2024

View reviewed changes

acceptance/testdata/auth/auth-switch.txtar Outdated Show resolved Hide resolved

jtmcg added 3 commits October 21, 2024 16:36

Setup acceptance testing for auth and tests for auth-token and auth-s…

665e814

…tatus

jtmcg force-pushed the jtmcg/testscripts-auth branch from f78a727 to 98d20e6 Compare October 21, 2024 23:48

jtmcg marked this pull request as ready for review October 21, 2024 23:48

jtmcg requested a review from a team as a code owner October 21, 2024 23:48

jtmcg requested a review from williammartin October 21, 2024 23:48

cliAutomation added the external pull request originating outside of the CLI core team label Oct 21, 2024

jtmcg removed the external pull request originating outside of the CLI core team label Oct 21, 2024

jtmcg force-pushed the jtmcg/testscripts-auth branch from 98d20e6 to 0be0fd2 Compare October 22, 2024 17:01

jtmcg changed the title ~~Jtmcg/testscripts auth~~ Add acceptance tests for gh auth commands Oct 22, 2024

williammartin requested changes Oct 23, 2024

View reviewed changes

jtmcg added 2 commits October 23, 2024 13:33

Remove comment from gh auth logout

59aedc4

jtmcg force-pushed the jtmcg/testscripts-auth branch from 6f06eac to 6f707c9 Compare October 23, 2024 20:56

Address PR feedback

2b480da

jtmcg force-pushed the jtmcg/testscripts-auth branch from 6f707c9 to 2b480da Compare October 23, 2024 20:58

jtmcg requested a review from williammartin October 23, 2024 21:09

williammartin approved these changes Oct 24, 2024

View reviewed changes

williammartin merged commit 2a2adfe into trunk Oct 24, 2024

williammartin deleted the jtmcg/testscripts-auth branch October 24, 2024 16:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add acceptance tests for `gh auth` commands #9787

Add acceptance tests for `gh auth` commands #9787

Uh oh!

jtmcg commented Oct 18, 2024 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

williammartin left a comment •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Add acceptance tests for gh auth commands #9787

Add acceptance tests for gh auth commands #9787

Uh oh!

Conversation

jtmcg commented Oct 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

gh auth status and gh auth token

gh auth setup-git

gh auth login

gh auth switch

Testing

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

williammartin left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Add acceptance tests for `gh auth` commands #9787

Add acceptance tests for `gh auth` commands #9787

jtmcg commented Oct 18, 2024 •

edited

Loading

`gh auth status` and `gh auth token`

`gh auth setup-git`

`gh auth login`

`gh auth switch`

williammartin left a comment •

edited

Loading