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

add sweepers auto-resolver code #12950

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ScottSuarez
Copy link
Contributor

Adding missing sweeper regions and introduce auto resolver code. Note that I haven't added unit tests since this code is still evolving. Need to work this for the {{name}} and {{prefixes}}.

Release Note Template for Downstream PRs (will be copied)

See Write release notes for guidance.


@modular-magician
Copy link
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 52 files changed, 247 insertions(+), 52 deletions(-))
google-beta provider: Diff ( 63 files changed, 286 insertions(+), 63 deletions(-))

@ScottSuarez ScottSuarez changed the title Add missing regions for sweepers and auto-resolver code add sweepers auto-resolver code Feb 3, 2025
@modular-magician
Copy link
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR hasn't generated any diffs, but I'll let you know if a future commit does.

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 1685
Passed tests: 1577
Skipped tests: 106
Affected tests: 2

Click here to see the affected service packages
  • bigqueryanalyticshub
  • bigqueryreservation
  • networkservices
  • notebooks
  • securitycenterv2
  • bigqueryconnection
  • certificatemanager
  • cloudbuild
  • networkconnectivity
  • cloudfunctions2
  • dataprocgdc
  • discoveryengine
  • integrations
  • networkmanagement
  • apphub
  • artifactregistry
  • biglake
  • iamworkforcepool
  • compute
  • documentai
  • filestore
  • networksecurity
  • redis
  • cloudrun
  • gkeonprem
  • parallelstore
  • cloudbuildv2
  • dialogflowcx
  • workbench

Action taken

Found 2 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
  • TestAccNetworkSecurityAuthzPolicy_networkServicesAuthzPolicyAdvancedExample
  • TestAccCloudbuildWorkerPool_basic

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

🔴 Tests failed during RECORDING mode:
TestAccCloudbuildWorkerPool_basic [Error message] [Debug log]
TestAccNetworkSecurityAuthzPolicy_networkServicesAuthzPolicyAdvancedExample [Error message] [Debug log]

🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR.

View the build log or the debug log for each test

log.Fatal("Please provide a directory path using -dir flag")
}

allTests, errs := reader.ReadAllTests(*dirPath)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally we'd want this to be integrated in the CI pipeline, which is already reading tests in order to do missing test detection. It would be great to only read them once, then do all the relevant processing of that data (for missing tests, sweeper regions, and sweepable prefix detection), and post any suggested changes as inline comments (or comments on the file in cases where inline doesn't work for some reason).

SourceFile string `yaml:"source_file,omitempty"`
}

func main() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should run this using cobra to match our other tooling and because it's more readable and abstracts away some of the weird stuff about golang CLI implementation. It should return the basic data about what issues were detected, which would get used by the CI to generate inline comments (ideally using some kind of generic implementation that could handle issue creation for missing test detector and sweepable prefixes as well - maybe also for other arbitrary feedback we might want to create in the future. For example, we could return an object that contains a message plus the line number and file to attach it to. Any multi-line message should most likely be built using a go template.)

@melinath
Copy link
Member

melinath commented Feb 3, 2025

This is pretty high level feedback so I'm not getting into details because I don't know how much will change - let me know if you have any questions!

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

Successfully merging this pull request may close these issues.

3 participants