-
Notifications
You must be signed in to change notification settings - Fork 350
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
eskip: refactor parsing tests to check error message #2874
Conversation
57e81cf
to
e9fc110
Compare
Check value of error message to simplify followup fixes. E.g. #1885 introduced last route id to the error message without tests but last route could be empty or should not be present at all for `ParsePredicates` and `ParseFilters`. Signed-off-by: Alexander Yastrebov <[email protected]>
e9fc110
to
0116f8e
Compare
}, { | ||
"error", | ||
"trallala", | ||
nil, | ||
true, | ||
// TODO: remove empty last route id and fix position |
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.
Position fix could be done by #2873
for _, ti := range []struct { | ||
msg string | ||
expression string | ||
check *Route | ||
err bool | ||
check []*Route |
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.
minor: maybe expectedRoutes would be better naming?
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.
I did not rename struct fields to keep diff small. All three tests use different naming
👍 |
1 similar comment
👍 |
Check value of error message to simplify followup fixes.
E.g. #1885 introduced last route id to the error message without tests but last route could be empty or should not be present at all for
ParsePredicates
andParseFilters
.