refactor: changed signature to error & enhanced warnings #57
Conversation
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
…r ApplyValidationRules ## Changes ### validators.go - Major Refactor - Changed Validator function signature from `func(string) bool` to `func(string) error` for better error logging with actual reasons - Refactored ApplyValidationRules to use clean helper methods instead of if-else chains: - `apply()` - dispatches to value or slice validation - `validateValue()` - validates single string values with early returns - `validateSlice()` - validates slice values - `normalizeValue()` / `normalizeSlice()` - handle normalization - `validate(value)` - runs validator or checks valid set, returns error - `printWarning()` - formats and prints warning with reason if available - Removed all `isValid` boolean variables for cleaner code ### parse_pattern.go - Validation Enhancements - Renamed `ValidatePattern` to `ValidateGroupPattern` for clarity - Added validation: pattern must contain xAxis (x) OR yAxis (y) - Added validation in `ParseBenchmarkNameWithRegex`: regex result must have xAxis or yAxis, otherwise returns error with clear message ### flag_validation_rules.go - Cleanup - Removed wrapper function, now directly uses `parser.ValidateGroupPattern` - Removed duplicate group regex validation rule ### Test Updates - Updated parse_pattern_test.go: - Test function now calls `ValidateGroupPattern` - Added test cases for xAxis/yAxis validation requirement - Added test cases for regex validation (name-only, xAxis-only, no named groups) - Updated flag_validation_test.go: removed group regex label assertion - Updated root_test.go: adjusted expected warning messages for new format ## Warning Message Format Old: Warning: Invalid time unit 'invalid'. Using default 'ns' New: Warning: Invalid time unit 'invalid'. Reason: value not in valid set [ns us ms s]. Using default 'ns'
✅ Deploy Preview for vizb-demo canceled.
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
- Add TestApplyValidationRulesSlice: valid slice, invalid slice fallback, slice with normalizer, empty slice handling - Add TestApplyValidationRulesWithCustomValidator: validator returning nil, validator returning error, specific error messages, slice with custom validator Coverage improved from 63.63% to 98.8% for validators.go
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.
validators.go - Major Refactor
func(string) booltofunc(string) errorfor better error logging with actual reasonsapply()- dispatches to value or slice validationvalidateValue()- validates single string values with early returnsvalidateSlice()- validates slice valuesnormalizeValue()/normalizeSlice()- handle normalizationvalidate(value)- runs validator or checks valid set, returns errorprintWarning()- formats and prints warning with reason if availableisValidboolean variables for cleaner codeparse_pattern.go - Validation Enhancements
ValidatePatterntoValidateGroupPatternfor clarityParseBenchmarkNameWithRegex: regex result must have xAxis or yAxis, otherwise returns error with clear messageflag_validation_rules.go - Cleanup
parser.ValidateGroupPatternTest Updates
ValidateGroupPatternWarning Message Format
Old: Warning: Invalid time unit 'invalid'. Using default 'ns' New: Warning: Invalid time unit 'invalid'. Reason: value not in valid set [ns us ms s]. Using default 'ns'