Skip to content

Conversation

@hslatman
Copy link
Member

@hslatman hslatman commented Jan 7, 2026

Broken test was merged as part of #1893.

@github-actions github-actions bot added the needs triage Waiting for discussion / prioritization by team label Jan 7, 2026
@hslatman hslatman force-pushed the herman/fix-root-not-found-tests branch from c45fd49 to 979edbb Compare January 7, 2026 21:08
@hslatman hslatman added this to the v0.29.1 milestone Jan 7, 2026
@hslatman hslatman requested review from a team, Copilot and marten-seemann January 7, 2026 21:17
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes broken tests related to root certificate not found error messages that were introduced in PR #1893. The tests were failing due to incorrect expected error message strings.

Changes

  • Updated expected error messages in tests to match the actual error format returned by the code
  • Fixed string escaping in test assertions to properly match quoted fingerprints in error messages

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
ca/ca_test.go Updated expected error message from generic default to specific fingerprint-based message
api/api_test.go Fixed string escaping in error message assertion to properly match quoted fingerprint

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

}{
{"ok", parseCertificate(rootPEM), nil, "", 200},
{"fail", nil, fmt.Errorf("not found"), fmt.Sprintf("root certificate with fingerprint %q was not found", sha), 404},
{"fail", nil, fmt.Errorf("not found"), fmt.Sprintf(`root certificate with fingerprint \"%s\" was not found`, sha), 404},
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

The manual escaping of quotes with backslashes is unnecessary and makes the test harder to read. Since the format string is already using backticks (raw string literal), you can include the quotes directly without escaping: root certificate with fingerprint \"%s\" was not found should be root certificate with fingerprint \"%s\" was not found or use double quotes with proper escaping if needed based on how the actual error message is formatted.

Suggested change
{"fail", nil, fmt.Errorf("not found"), fmt.Sprintf(`root certificate with fingerprint \"%s\" was not found`, sha), 404},
{"fail", nil, fmt.Errorf("not found"), fmt.Sprintf(`root certificate with fingerprint "%s" was not found`, sha), 404},

Copilot uses AI. Check for mistakes.
Copy link
Member Author

@hslatman hslatman Jan 7, 2026

Choose a reason for hiding this comment

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

Sorry, but that is the wrong conclusion, Copilot.

@hslatman hslatman changed the title Fix tests for root not found by fingerprint tests Fix tests for root not found by fingerprint functionality Jan 7, 2026
@hslatman hslatman changed the title Fix tests for root not found by fingerprint functionality Fix tests for root not found by fingerprint Jan 8, 2026
@hslatman hslatman requested a review from maraino January 8, 2026 10:35
@hslatman hslatman enabled auto-merge January 8, 2026 16:42
Copy link
Contributor

@tashian tashian left a comment

Choose a reason for hiding this comment

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

lgtm

@hslatman hslatman merged commit e6c864f into master Jan 8, 2026
14 checks passed
@hslatman hslatman deleted the herman/fix-root-not-found-tests branch January 8, 2026 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs triage Waiting for discussion / prioritization by team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants