🧪 fix: Logger Middleware tests to use regex for time validation#3392
🧪 fix: Logger Middleware tests to use regex for time validation#3392ReneWerner87 merged 2 commits intomainfrom
Conversation
WalkthroughThe pull request refactors the test cases in the Changes
Sequence Diagram(s)sequenceDiagram
participant T as Test Case
participant L as Logger Module
T->>T: Initialize HTTP parameter constants
T->>L: Invoke logging function with parameters
L-->>T: Return formatted log message
T->>T: Validate log output using regex matching
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3392 +/- ##
==========================================
- Coverage 83.91% 83.87% -0.05%
==========================================
Files 119 119
Lines 11904 11904
==========================================
- Hits 9989 9984 -5
- Misses 1486 1490 +4
- Partials 429 430 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
middleware/logger/logger_test.go (4)
492-493: Improved test reliability with regex for time validation.This change is the core of the PR objective - replacing direct time comparison with regex pattern matching. The pattern now correctly validates the log format while being resilient to time differences.
However, there's an opportunity for a small optimization:
-pattern := fmt.Sprintf(`0\.0\.0\.0 - - \[\d{2}:\d{2}:\d{2}\] "%s %s %s" %d %d`, method, regexp.QuoteMeta(path), proto, status, bytesSent) -require.Regexp(t, regexp.MustCompile(pattern), buf.String()) +pattern := regexp.MustCompile(fmt.Sprintf(`0\.0\.0\.0 - - \[\d{2}:\d{2}:\d{2}\] "%s %s %s" %d %d`, method, regexp.QuoteMeta(path), proto, status, bytesSent)) +require.Regexp(t, pattern, buf.String())🧰 Tools
🪛 GitHub Check: lint
[failure] 493-493:
regexp: remove unnecessary regexp.MustCompile (testifylint)
523-524: Regex pattern for combined format with the same optimization opportunity.The regex approach works well for this test case too, allowing for time variability.
-pattern := fmt.Sprintf(`0\.0\.0\.0 - - \[\d{2}:\d{2}:\d{2}\] "%s %s %s" %d %d "%s" "%s"`, method, regexp.QuoteMeta(path), proto, status, bytesSent, regexp.QuoteMeta(referer), regexp.QuoteMeta(ua)) -require.Regexp(t, regexp.MustCompile(pattern), buf.String()) +pattern := regexp.MustCompile(fmt.Sprintf(`0\.0\.0\.0 - - \[\d{2}:\d{2}:\d{2}\] "%s %s %s" %d %d %q %q`, method, regexp.QuoteMeta(path), proto, status, bytesSent, regexp.QuoteMeta(referer), regexp.QuoteMeta(ua))) +require.Regexp(t, pattern, buf.String())🧰 Tools
🪛 GitHub Check: lint
[failure] 524-524:
regexp: remove unnecessary regexp.MustCompile (testifylint)
[failure] 523-523:
sprintfQuotedString: use %q instead of "%s" for quoted strings (gocritic)
549-550: JSON format regex pattern matching.The pattern correctly validates the JSON structure while being flexible with time format.
-pattern := fmt.Sprintf(`\{"time":"\d{2}:\d{2}:\d{2}","ip":"%s","method":"%s","url":"%s","status":%d,"bytesSent":%d\}`, regexp.QuoteMeta(ip), method, regexp.QuoteMeta(path), status, bytesSent) -require.Regexp(t, regexp.MustCompile(pattern), buf.String()) +pattern := regexp.MustCompile(fmt.Sprintf(`\{"time":"\d{2}:\d{2}:\d{2}","ip":%q,"method":%q,"url":%q,"status":%d,"bytesSent":%d\}`, regexp.QuoteMeta(ip), method, regexp.QuoteMeta(path), status, bytesSent)) +require.Regexp(t, pattern, buf.String())🧰 Tools
🪛 GitHub Check: lint
[failure] 550-550:
regexp: remove unnecessary regexp.MustCompile (testifylint)
[failure] 549-549:
sprintfQuotedString: use %q instead of "%s" for quoted strings (gocritic)
577-578: ECS format regex validation.The pattern correctly validates the complex ECS JSON structure while being time-resilient.
-pattern := fmt.Sprintf(`\{"@timestamp":"\d{2}:\d{2}:\d{2}","ecs":\{"version":"1.6.0"\},"client":\{"ip":"%s"\},"http":\{"request":\{"method":"%s","url":"%s","protocol":"%s"\},"response":\{"status_code":%d,"body":\{"bytes":%d\}\}\},"log":\{"level":"INFO","logger":"fiber"\},"message":"%s"\}`, regexp.QuoteMeta(ip), method, regexp.QuoteMeta(path), proto, status, bytesSent, regexp.QuoteMeta(msg)) -require.Regexp(t, regexp.MustCompile(pattern), buf.String()) +pattern := regexp.MustCompile(fmt.Sprintf(`\{"@timestamp":"\d{2}:\d{2}:\d{2}","ecs":\{"version":"1.6.0"\},"client":\{"ip":%q\},"http":\{"request":\{"method":%q,"url":%q,"protocol":%q\},"response":\{"status_code":%d,"body":\{"bytes":%d\}\}\},"log":\{"level":"INFO","logger":"fiber"\},"message":%q\}`, regexp.QuoteMeta(ip), method, regexp.QuoteMeta(path), proto, status, bytesSent, regexp.QuoteMeta(msg))) +require.Regexp(t, pattern, buf.String())🧰 Tools
🪛 GitHub Check: lint
[failure] 578-578:
regexp: remove unnecessary regexp.MustCompile (testifylint)
[failure] 577-577:
sprintfQuotedString: use %q instead of "%s" for quoted strings (gocritic)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
middleware/logger/logger_test.go(5 hunks)
🧰 Additional context used
🪛 GitHub Check: lint
middleware/logger/logger_test.go
[failure] 493-493:
regexp: remove unnecessary regexp.MustCompile (testifylint)
[failure] 484-484:
string HTTP/1.1 has 3 occurrences, make it a constant (goconst)
[failure] 483-483:
string /?foo=bar has 4 occurrences, make it a constant (goconst)
[failure] 524-524:
regexp: remove unnecessary regexp.MustCompile (testifylint)
[failure] 523-523:
sprintfQuotedString: use %q instead of "%s" for quoted strings (gocritic)
[failure] 550-550:
regexp: remove unnecessary regexp.MustCompile (testifylint)
[failure] 549-549:
sprintfQuotedString: use %q instead of "%s" for quoted strings (gocritic)
[failure] 578-578:
regexp: remove unnecessary regexp.MustCompile (testifylint)
[failure] 577-577:
sprintfQuotedString: use %q instead of "%s" for quoted strings (gocritic)
🪛 GitHub Actions: golangci-lint
middleware/logger/logger_test.go
[warning] 483-483: string /?foo=bar has 4 occurrences, make it a constant (goconst)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: unit (1.24.x, ubuntu-latest)
- GitHub Check: unit (1.24.x, windows-latest)
- GitHub Check: unit (1.23.x, windows-latest)
- GitHub Check: repeated
- GitHub Check: Analyse
- GitHub Check: Compare
🔇 Additional comments (10)
middleware/logger/logger_test.go (10)
14-14: Added regexp package for time validation.The import of the regexp package aligns with the PR objective to improve logger format tests by using regex for time validation instead of direct time comparisons.
482-487: Good refactoring: introduced variables for common test values.Extracting these values into variables improves readability and maintainability. When test configurations need to change, they can be modified in one place rather than throughout the code.
🧰 Tools
🪛 GitHub Check: lint
[failure] 484-484:
stringHTTP/1.1has 3 occurrences, make it a constant (goconst)
[failure] 483-483:
string/?foo=barhas 4 occurrences, make it a constant (goconst)🪛 GitHub Actions: golangci-lint
[warning] 483-483: string
/?foo=barhas 4 occurrences, make it a constant (goconst)
488-490: Using variables in test request and assertions.The code now uses the defined variables consistently in both the test request and assertions, which improves consistency and reduces potential errors.
507-514: Good separation of test data in Combined CLF Format test.Defining these variables separately makes the test more readable and maintainable, especially since this test has more parameters than the previous one.
515-521: Well-structured request setup with headers.The approach of setting up the request with headers using variables is clean and easy to understand.
538-543: JSON format test variables clearly defined.Good use of variables to prepare the test data for JSON format testing.
544-547: Clean request setup and assertion for JSON format.The test is structured consistently with the other format tests.
564-571: Comprehensive test data setup for ECS format.All necessary variables for the ECS format test are clearly defined with descriptive names.
572-575: Request setup and status assertion for ECS format.The pattern follows the established structure from previous tests.
482-487:Details
✅ Verification successful
Consider creating constants for repeated test values.
Several values like
fiber.MethodGet, path, and protocol are used across multiple tests. Consider extracting these to constants to further improve maintainability.const ( testPath = "/?foo=bar" testProtocol = "HTTP/1.1" testReferer = "http://example.com" testUA = "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/74.0.3729.169 Safari/537.36" testIP = "0.0.0.0" )Also applies to: 507-514, 538-543, 564-571
🏁 Script executed:
#!/bin/bash # Search for the string literals that appear multiple times echo "Occurrences of '/?foo=bar':" rg -c "/?foo=bar" --no-filename echo -e "\nOccurrences of 'HTTP/1.1':" rg -c "HTTP/1.1" --no-filenameLength of output: 248
Extract constants for repeated test literals to enhance maintainability
Our recent verification confirms that literals such as
"/?foo=bar"and"HTTP/1.1"appear repeatedly across multiple test files (as indicated by the occurrence counts). To reduce duplication and mitigate potential maintenance issues, please consider extracting these repeated values into constants. For example:const ( testPath = "/?foo=bar" testProtocol = "HTTP/1.1" testReferer = "http://example.com" testUA = "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/74.0.3729.169 Safari/537.36" testIP = "0.0.0.0" )Impacted Code Locations:
middleware/logger/logger_test.golines 482-487, as well as similar test sections around lines 507-514, 538-543, and 564-571.🧰 Tools
🪛 GitHub Check: lint
[failure] 484-484:
stringHTTP/1.1has 3 occurrences, make it a constant (goconst)
[failure] 483-483:
string/?foo=barhas 4 occurrences, make it a constant (goconst)🪛 GitHub Actions: golangci-lint
[warning] 483-483: string
/?foo=barhas 4 occurrences, make it a constant (goconst)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
middleware/logger/logger_test.go (6)
492-493: Replaced fixed string comparison with regex pattern for time validation.This change addresses the PR objective by using a regex pattern to validate the time format rather than comparing exact time values, which will prevent flaky tests caused by time variations.
Consider removing the unnecessary
regexp.MustCompilecall as suggested by the linter:-require.Regexp(t, regexp.MustCompile(pattern), buf.String()) +require.Regexp(t, pattern, buf.String())🧰 Tools
🪛 GitHub Check: lint
[failure] 493-493:
regexp: remove unnecessary regexp.MustCompile (testifylint)
507-523: Enhanced Combined CLF Format test with extracted variables and improved setup.The refactoring for the Combined CLF Format test follows the same pattern, with clear variable declarations and improved request setup including referer and user-agent headers.
In the pattern string construction at line 523, consider using
%qinstead of"%s"for quoted strings as suggested by the linter:-pattern := fmt.Sprintf(`0\.0\.0\.0 - - \[\d{2}:\d{2}:\d{2}\] "%s %s %s" %d %d "%s" "%s"`, method, regexp.QuoteMeta(path), proto, status, bytesSent, regexp.QuoteMeta(referer), regexp.QuoteMeta(ua)) +pattern := fmt.Sprintf(`0\.0\.0\.0 - - \[\d{2}:\d{2}:\d{2}\] %q %q %q %d %d %q %q`, method, regexp.QuoteMeta(path), proto, status, bytesSent, regexp.QuoteMeta(referer), regexp.QuoteMeta(ua))🧰 Tools
🪛 GitHub Check: lint
[failure] 523-523:
sprintfQuotedString: use %q instead of "%s" for quoted strings (gocritic)
524-524: Same suggestion for regexp.MustCompile as earlier test.Similar to the previous test, the unnecessary
regexp.MustCompilecall could be removed.-require.Regexp(t, regexp.MustCompile(pattern), buf.String()) +require.Regexp(t, pattern, buf.String())🧰 Tools
🪛 GitHub Check: lint
[failure] 524-524:
regexp: remove unnecessary regexp.MustCompile (testifylint)
538-550: Improved JSON Format test with variables and regex pattern.The JSON Format test has been similarly refactored with clear variable declarations and regex matching for time validation, making it more resilient to time-related test flakiness.
Same suggestions apply here:
- Remove unnecessary
regexp.MustCompile- Consider using
%qfor quoted strings in the pattern-pattern := fmt.Sprintf(`\{"time":"\d{2}:\d{2}:\d{2}","ip":"%s","method":"%s","url":"%s","status":%d,"bytesSent":%d\}`, regexp.QuoteMeta(ip), method, regexp.QuoteMeta(path), status, bytesSent) +pattern := fmt.Sprintf(`\{"time":"\d{2}:\d{2}:\d{2}","ip":%q,"method":%q,"url":%q,"status":%d,"bytesSent":%d\}`, regexp.QuoteMeta(ip), method, regexp.QuoteMeta(path), status, bytesSent) -require.Regexp(t, regexp.MustCompile(pattern), buf.String()) +require.Regexp(t, pattern, buf.String())🧰 Tools
🪛 GitHub Check: lint
[failure] 550-550:
regexp: remove unnecessary regexp.MustCompile (testifylint)
[failure] 549-549:
sprintfQuotedString: use %q instead of "%s" for quoted strings (gocritic)
564-578: Enhanced ECS Format test with detailed variables and regex pattern.The ECS Format test has been comprehensively refactored with clear variables for all components and a regex pattern for time validation, ensuring more reliable test execution.
Same suggestions apply here as well:
- Remove unnecessary
regexp.MustCompile- Consider using
%qfor quoted strings in the pattern-pattern := fmt.Sprintf(`\{"@timestamp":"\d{2}:\d{2}:\d{2}","ecs":\{"version":"1.6.0"\},"client":\{"ip":"%s"\},"http":\{"request":\{"method":"%s","url":"%s","protocol":"%s"\},"response":\{"status_code":%d,"body":\{"bytes":%d\}\}\},"log":\{"level":"INFO","logger":"fiber"\},"message":"%s"\}`, regexp.QuoteMeta(ip), method, regexp.QuoteMeta(path), proto, status, bytesSent, regexp.QuoteMeta(msg)) +pattern := fmt.Sprintf(`\{"@timestamp":"\d{2}:\d{2}:\d{2}","ecs":\{"version":"1.6.0"\},"client":\{"ip":%q\},"http":\{"request":\{"method":%q,"url":%q,"protocol":%q\},"response":\{"status_code":%d,"body":\{"bytes":%d\}\}\},"log":\{"level":"INFO","logger":"fiber"\},"message":%q\}`, regexp.QuoteMeta(ip), method, regexp.QuoteMeta(path), proto, status, bytesSent, regexp.QuoteMeta(msg)) -require.Regexp(t, regexp.MustCompile(pattern), buf.String()) +require.Regexp(t, pattern, buf.String())🧰 Tools
🪛 GitHub Check: lint
[failure] 578-578:
regexp: remove unnecessary regexp.MustCompile (testifylint)
[failure] 577-577:
sprintfQuotedString: use %q instead of "%s" for quoted strings (gocritic)
483-483: Consider extracting common constants.Several test functions use the same values for method, path, and protocol. Consider extracting these into constants to improve maintainability.
+const ( + testMethod = fiber.MethodGet + testPath = "/?foo=bar" + testProto = "HTTP/1.1" +) // Then use these constants in each test function -method := fiber.MethodGet -path := "/?foo=bar" -proto := "HTTP/1.1" +method := testMethod +path := testPath +proto := testProtoAlso applies to: 507-509
🧰 Tools
🪛 GitHub Check: lint
[failure] 483-483:
string/?foo=barhas 4 occurrences, make it a constant (goconst)🪛 GitHub Actions: golangci-lint
[warning] 483-483: string
/?foo=barhas 4 occurrences, make it a constant (goconst)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
middleware/logger/logger_test.go(5 hunks)
🧰 Additional context used
🪛 GitHub Check: lint
middleware/logger/logger_test.go
[failure] 493-493:
regexp: remove unnecessary regexp.MustCompile (testifylint)
[failure] 484-484:
string HTTP/1.1 has 3 occurrences, make it a constant (goconst)
[failure] 483-483:
string /?foo=bar has 4 occurrences, make it a constant (goconst)
[failure] 524-524:
regexp: remove unnecessary regexp.MustCompile (testifylint)
[failure] 523-523:
sprintfQuotedString: use %q instead of "%s" for quoted strings (gocritic)
[failure] 550-550:
regexp: remove unnecessary regexp.MustCompile (testifylint)
[failure] 549-549:
sprintfQuotedString: use %q instead of "%s" for quoted strings (gocritic)
[failure] 578-578:
regexp: remove unnecessary regexp.MustCompile (testifylint)
[failure] 577-577:
sprintfQuotedString: use %q instead of "%s" for quoted strings (gocritic)
🪛 GitHub Actions: golangci-lint
middleware/logger/logger_test.go
[warning] 483-483: string /?foo=bar has 4 occurrences, make it a constant (goconst)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: repeated
- GitHub Check: Compare
🔇 Additional comments (3)
middleware/logger/logger_test.go (3)
14-14: Added regexp import for regex-based time validation.The addition of the
regexppackage allows the implementation of more robust log verification patterns that can handle time variations in log outputs.
482-487: Good refactoring: Extracted variables for better maintainability.Introducing variables for method, path, protocol, status, and bytesSent improves code readability and makes the test easier to modify and maintain.
🧰 Tools
🪛 GitHub Check: lint
[failure] 484-484:
stringHTTP/1.1has 3 occurrences, make it a constant (goconst)
[failure] 483-483:
string/?foo=barhas 4 occurrences, make it a constant (goconst)🪛 GitHub Actions: golangci-lint
[warning] 483-483: string
/?foo=barhas 4 occurrences, make it a constant (goconst)
488-490: Improved test setup with extracted variables.Using the predefined variables in the HTTP request creation and assertion logic makes the code more consistent and reduces the risk of errors from mismatched values.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
middleware/logger/logger_test.go (4)
482-493: Good refactoring to make test more maintainable and less flaky.The introduction of explicit variables for HTTP parameters (method, path, protocol, status, bytesSent) instead of hardcoded values improves readability and maintainability. The use of regex for time validation is an excellent approach to handle dynamic timestamps and avoid flaky tests.
Consider making frequently used string literals into constants as flagged by the linter:
// At the package level +const ( + testPath = "/?foo=bar" + testProto = "HTTP/1.1" +) // Then in the test - path := "/?foo=bar" - proto := "HTTP/1.1" + path := testPath + proto := testProtoAlso, you can optimize the regex compilation by removing the unnecessary
MustCompile:- require.Regexp(t, regexp.MustCompile(pattern), buf.String()) + require.Regexp(t, pattern, buf.String())🧰 Tools
🪛 GitHub Check: lint
[failure] 493-493:
regexp: remove unnecessary regexp.MustCompile (testifylint)
[failure] 484-484:
stringHTTP/1.1has 3 occurrences, make it a constant (goconst)
[failure] 483-483:
string/?foo=barhas 4 occurrences, make it a constant (goconst)🪛 GitHub Actions: golangci-lint
[warning] 483-483: string
/?foo=barhas 4 occurrences, make it a constant (goconst)
507-524: Well-structured test with improved test parameters and regex validation.The test now explicitly defines all HTTP parameters and uses regex to validate timestamp formats in the log output. This approach makes the test more robust against timing-related flakiness.
Similar to the previous test, consider:
- Using constants for repeated string literals
- Removing unnecessary
regexp.MustCompile- Using
%qinstead of"%s"for quoted strings in sprintf as recommended by the linter:- pattern := fmt.Sprintf(`0\.0\.0\.0 - - \[\d{2}:\d{2}:\d{2}\] "%s %s %s" %d %d "%s" "%s"`, method, regexp.QuoteMeta(path), proto, status, bytesSent, regexp.QuoteMeta(referer), regexp.QuoteMeta(ua)) + pattern := fmt.Sprintf(`0\.0\.0\.0 - - \[\d{2}:\d{2}:\d{2}\] %q %s %s" %d %d %q %q`, method, regexp.QuoteMeta(path), proto, status, bytesSent, regexp.QuoteMeta(referer), regexp.QuoteMeta(ua))- require.Regexp(t, regexp.MustCompile(pattern), buf.String()) + require.Regexp(t, pattern, buf.String())🧰 Tools
🪛 GitHub Check: lint
[failure] 524-524:
regexp: remove unnecessary regexp.MustCompile (testifylint)
[failure] 523-523:
sprintfQuotedString: use %q instead of "%s" for quoted strings (gocritic)
538-550: Improved JSON format test with clear variable declarations.Using explicit variables for HTTP parameters makes the test more readable and maintainable. The regex pattern properly accounts for dynamic timestamps while ensuring the JSON structure is correctly validated.
Similar to previous suggestions:
- pattern := fmt.Sprintf(`\{"time":"\d{2}:\d{2}:\d{2}","ip":"%s","method":"%s","url":"%s","status":%d,"bytesSent":%d\}`, regexp.QuoteMeta(ip), method, regexp.QuoteMeta(path), status, bytesSent) + pattern := fmt.Sprintf(`\{"time":"\d{2}:\d{2}:\d{2}","ip":%q,"method":%q,"url":%q,"status":%d,"bytesSent":%d\}`, regexp.QuoteMeta(ip), method, regexp.QuoteMeta(path), status, bytesSent)- require.Regexp(t, regexp.MustCompile(pattern), buf.String()) + require.Regexp(t, pattern, buf.String())🧰 Tools
🪛 GitHub Check: lint
[failure] 550-550:
regexp: remove unnecessary regexp.MustCompile (testifylint)
[failure] 549-549:
sprintfQuotedString: use %q instead of "%s" for quoted strings (gocritic)
564-578: Well-structured ECS format test with comprehensive parameter handling.This refactoring improves test maintainability by using explicit variables and creating a clear message expectation. The regex pattern correctly handles dynamic timestamps while ensuring the ECS format is validated.
Apply similar improvements as in previous tests:
- pattern := fmt.Sprintf(`\{"@timestamp":"\d{2}:\d{2}:\d{2}","ecs":\{"version":"1.6.0"\},"client":\{"ip":"%s"\},"http":\{"request":\{"method":"%s","url":"%s","protocol":"%s"\},"response":\{"status_code":%d,"body":\{"bytes":%d\}\}\},"log":\{"level":"INFO","logger":"fiber"\},"message":"%s"\}`, regexp.QuoteMeta(ip), method, regexp.QuoteMeta(path), proto, status, bytesSent, regexp.QuoteMeta(msg)) + pattern := fmt.Sprintf(`\{"@timestamp":"\d{2}:\d{2}:\d{2}","ecs":\{"version":"1.6.0"\},"client":\{"ip":%q\},"http":\{"request":\{"method":%q,"url":%q,"protocol":%q\},"response":\{"status_code":%d,"body":\{"bytes":%d\}\}\},"log":\{"level":"INFO","logger":"fiber"\},"message":%q\}`, regexp.QuoteMeta(ip), method, regexp.QuoteMeta(path), proto, status, bytesSent, regexp.QuoteMeta(msg))- require.Regexp(t, regexp.MustCompile(pattern), buf.String()) + require.Regexp(t, pattern, buf.String())🧰 Tools
🪛 GitHub Check: lint
[failure] 578-578:
regexp: remove unnecessary regexp.MustCompile (testifylint)
[failure] 577-577:
sprintfQuotedString: use %q instead of "%s" for quoted strings (gocritic)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
middleware/logger/logger_test.go(5 hunks)
🧰 Additional context used
🪛 GitHub Check: lint
middleware/logger/logger_test.go
[failure] 493-493:
regexp: remove unnecessary regexp.MustCompile (testifylint)
[failure] 484-484:
string HTTP/1.1 has 3 occurrences, make it a constant (goconst)
[failure] 483-483:
string /?foo=bar has 4 occurrences, make it a constant (goconst)
[failure] 524-524:
regexp: remove unnecessary regexp.MustCompile (testifylint)
[failure] 523-523:
sprintfQuotedString: use %q instead of "%s" for quoted strings (gocritic)
[failure] 550-550:
regexp: remove unnecessary regexp.MustCompile (testifylint)
[failure] 549-549:
sprintfQuotedString: use %q instead of "%s" for quoted strings (gocritic)
[failure] 578-578:
regexp: remove unnecessary regexp.MustCompile (testifylint)
[failure] 577-577:
sprintfQuotedString: use %q instead of "%s" for quoted strings (gocritic)
🪛 GitHub Actions: golangci-lint
middleware/logger/logger_test.go
[warning] 483-483: string /?foo=bar has 4 occurrences, make it a constant (goconst)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: repeated
- GitHub Check: Compare
🔇 Additional comments (5)
middleware/logger/logger_test.go (5)
14-14: Appropriate addition of the regexp package.The addition of the regexp package aligns perfectly with the PR objectives of using regex for time validation in logger format tests, which will help reduce flaky tests.
489-490: Consistent variable usage for status code validation.The change from hardcoded status code to using the
statusvariable makes the test more consistent and maintainable.
521-522: Consistent status code validation.Using the
statusvariable for validation instead of hardcoded values improves code consistency.
547-548: Consistent status code validation.Using the
statusvariable for validation provides consistency with the test setup.
575-576: Consistent status code validation.Using the
statusvariable for validation provides consistency with the test setup.
There was a problem hiding this comment.
Pull Request Overview
This PR updates the logger middleware tests to validate the timestamp using regex rather than comparing precise time strings, reducing test flakiness. Key changes include:
- Importing the "regexp" package and defining constants for common values.
- Replacing exact string comparisons with regex-based validations for CLF, Combined CLF, JSON, and ECS log formats.
- Refactoring tests to use predefined constants (e.g., pathFooBar, httpProto) for consistency.
…ber#3392) * test: rewrite logger format tests to use regex for time validation * test: rewrite logger format tests to use regex for time validation
to reduce flaky test run, the time is no longer compared, but the format of the time which is in the logger format is looked at
example:
https://github.com/gofiber/fiber/actions/runs/14219745651/job/39845609228?pr=3391