- 
                Notifications
    You must be signed in to change notification settings 
- Fork 17
          query: refactor query test step logic into testStepNewQuery()
          #568
        
          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
Conversation
RunQueryChecks()RunQueryChecks()
      RunQueryChecks()testStepNewQuery()
      There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one non-blocking note, code LGTM 👍🏻
| } | ||
| } | ||
| `, | ||
| ExpectError: regexp.MustCompile(`Diagnostic summary`), | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something worth noting that might not be an immediate problem, but more of a potential future confusion for folks familiar with
ExpectError
Since Query is using the machine readable output, the error message it is running the regex on with that mode is the formatted %+v of the diagnostics, which is different then other modes that use the formatted Terraform CLI output of the diagnostics. That can make the output hard to read / different to match than the other modes:
--- FAIL: TestQuery_ExpectError_ValidationError (0.49s)
    /Users/austin.valle/code/terraform-plugin-testing/helper/resource/query/query_test.go:111: Step 2/2 error running query, expected an error with pattern (Not the correct diagnostic summary), no match on: running terraform query command returned diagnostics: [{baseLogMessage:{Lvl:error Msg:Error: Diagnostic summary Time:2025-10-29 09:10:02.151561 -0400 EDT} Diagnostic:{Severity:error Summary:Diagnostic summary Detail:Diagnostic details Range:0x140004547c0 Snippet:0x14000030640}} {baseLogMessage:{Lvl:error Msg:Error: Diagnostic summary Time:2025-10-29 09:10:02.151735 -0400 EDT} Diagnostic:{Severity:error Summary:Diagnostic summary Detail:Diagnostic details Range:0x14000454900 Snippet:0x14000030690}}]There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that's a good point. Maybe we should introduce a string formatter for machine readable error messaging in a follow-up PR.
Related Issue
Relates: #561
Description
Refactors query test step logic into
testStepNewQuery()allowing query errors to be used withExpectError.Rollback Plan
Changes to Security Controls
Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.
N/A