Repository navigation
test(cli): stop the onboarding tests from running the real gcloud - #7881
Open
adityasingh2400 wants to merge 1 commit into
Open
adityasingh2400 wants to merge 1 commit into
adityasingh2400 wants to merge 1 commit into
Conversation
The autouse mock_gcp_metadata_off fixture hides Application Default Credentials, so test_handle_login_with_google_option_3, which didn't mock check_adc, always ran the real `gcloud auth application-default login`: on a machine with gcloud installed every test run opened a browser sign-in, and on CI it only passed because the missing gcloud happened to raise click.Abort. test_handle_login_with_google_option_2 read the real default project and, with click.confirm mocked to True, ran the real `gcloud config unset project`. Mock both calls, and add an autouse guard that fails any test in the module that would run the gcloud CLI.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Please ensure you have read the contribution guide before creating a pull request.
Link to Issue or Description of Change
1. Link to an existing issue (if applicable):
Problem:
On a developer machine with gcloud installed, two onboarding tests in
tests/unittests/cli/utils/test_cli_create.pyran the real gcloud CLI.test_handle_login_with_google_option_3didn't mockgcp_utils.check_adc. The autousemock_gcp_metadata_offfixture hides Application Default Credentials, socheck_adc()is always False in the suite. Every run therefore calledlogin_adc(), i.e.gcloud auth application-default login, which opens a browser sign-in. On CI, gcloud is missing, so theclick.Abortthe test expects came from the failed login rather than from choosing "3".test_handle_login_with_google_option_2didn't mock_onboarding.get_gcp_project_from_gcloud. It read the real default project and, sinceclick.confirmis mocked toTrue, rangcloud config unset projecton the developer's configuration.Solution:
option_3mockscheck_adclike the otherhandle_login_with_googletests, so it now reaches and covers the "3. Abandon" choice.option_2mocksget_gcp_project_from_gcloud(no default project), asoption_2_unset_projectalready does with its own value._no_real_gcloudfixture makessubprocess.runraise anAssertionErrorfor anygcloudcommand. A future test that forgets a mock then fails loudly instead of touching the developer's machine. Tests that patchsubprocess.runthemselves (e.g. theunset_projectand*_from_gcloud_failtests) still override it.Test-only change; no library code is touched.
Testing Plan
Unit Tests:
With the guard alone, without the two mocks, the guard names exactly the two offenders:
With the fix, I put a logging fake
gcloudfirst onPATHto catch anything that slips through:test_cli_create.pydrops from about 9.6s to 0.3s, since nothing waits on gcloud any more.pre-commit runpasses on the changed file.Manual End-to-End (E2E) Tests:
Before the fix, gcloud's own log (
~/.config/gcloud/logs/) from running the suite showed the default project being unset, and a new ADC login on every run:After the fix, running the CLI tests adds no gcloud log entries.
Checklist
Additional context
None.